This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Some things are only available in JUnit module. For example the "Create tests" action and GUI as well as the "Run/Debug Focused Method" actions. In order for them to be re-usable from other modules (like testng) they should be made available through common.testrunner with which there is already a friend dependency from junit and testng Branch TestNG_2012 is already created under main. The changesets that contain the registration of the actions and the appropriate GUI in testrunner and the implementation of specific Providers in junit can be found here http://hg.netbeans.org/main/rev/c21292347fca and http://hg.netbeans.org/main/rev/8904f692d00e I will attach them as diff as well
Created attachment 116092 [details] TestNG_2012 branch - initial commit
Created attachment 116093 [details] TestNG_2012 branch
Jarda is interested in this too
I don't see a module called common.testrunner neither in the branch nor in the trunk - did you mean gsf.testrunner?
(In reply to comment #4) > I don't see a module called common.testrunner neither in the branch nor in the > trunk - did you mean gsf.testrunner? yes gsf.testrunner is what I meant to write, sorry for that.
Next time please include the BZ number in the branch name, e.g. "testng_208841". Also too late now, but next time you want to move modules between repositories, do it with history: http://wiki.netbeans.org/HgHowTos#Transfer_a_module.27s_history_to_another_repository Also some thing look like they were renames, but are not marked in history this way; junit/src/org/netbeans/modules/junit/JUnitCfgOfCreate.java -> gsf.testrunner/src/org/netbeans/modules/gsf/testrunner/CommonTestsCfgOfCreate.java for example. Also do not comment things out in layer.xml files; it makes the diff quite confusing and inevitably leaves trash behind for years. If an entry is no longer needed, delete it. [JG01] Suggest the package prefix org.netbeans.modules.testng, not org.netbeans.modules.contrib.testng. BTW do not forget that new modules must be added to some plugin or another, e.g. java.kit. [JG02] gsf.testrunner may not depend on api.java nor java.project modules, because it is in the ide cluster. Perhaps you meant to add new APIs to java.project or some new module in the java cluster ("java.tests"?), rather than cluttering gsf.testrunner with things specific to the Java language which would not be used e.g. by Ruby support. Fixing this will require a big rearrangement of sources, so there is not much sense reviewing the branch further until this is done. [JG03] The "API" looks like it is packed with implementation classes which do not belong there. What is the use case for a client referring directly to org.netbeans.modules.gsf.testrunner.api.TestCreatorAction, for example? [JG04] Suggest moving the "TestNG-6.4beta" library, as well as testng-6.4beta-javadoc.zip, to testng.ant, moving the addLibrary method to AntTestNGSupport, and making MavenTestNGSupport.configureProject directly add the desired POM dependency.
re [JG01]: - removed 'contrib' from package name: http://hg.netbeans.org/main/rev/69f5314b6791 - added common & ant related stuff to java.kit: http://hg.netbeans.org/main/rev/62231b93f8e8 -added mvn part to maven.kit: http://hg.netbeans.org/main/rev/7bd4160f1ff3
re [JG02]: created new module java.testrunner in java cluster and removed api.java and java.project dependencies from gsf.testrunner: http://hg.netbeans.org/main/rev/089d80d01d8c
Re. JG04 - I'd like to slowly experiment with rewriting some of the NetBeans tests to TestNG. For that it will be beneficial to have separate module libs.testng. testng.ant should then depend on it.
Y01 Please change the TestCreatorProvider to declarative registration so you can show the list of registered providers without loading their implementation classes. Compile time annotation @TestCreator.Registration(displayName=...) would work well. Y02 TestCreatorProvider has setters and that seems strange. The provider is global object registered in Lookup. The setters change its global state and may or may not persist between multiple invocations depending on behavior of garbage collector (as the Lookup holds its instances via weak references). Y03 Ad. Y02. Consider creating TestCreate.Context that would have the setters as well as Nodes to work on. createTests method would accept Context as its only parameter.
(In reply to comment #9) > Re. JG04 - [..for] rewriting some of the NetBeans > tests to TestNG [...] it will be beneficial to have separate module > libs.testng You mean to use a <test-dependency>. Yes, this makes sense, since TestNG can produce a compatible XML result format. It would need to be in the platform cluster, and currently testng (not just testng.ant) would need to depend on it. Beware that there is compatibility code in the harness which adds a libs.junit4 dep if none is given explicitly; this would need to be amended to check for libs.testng too. Obviously you would have a lot of work to do if you wanted to use nbjunit features like workDir, logLevel, NbModuleSuite, @RandomlyFails, AbstractWhateverTestHid, etc.
Fixed [Y01], [Y02] and [Y03]: http://hg.netbeans.org/main/rev/6cb9202dafdc
(In reply to comment #12) > Fixed [Y01], [Y02] and [Y03]: http://hg.netbeans.org/main/rev/6cb9202dafdc Opps, I silently assumed that TestCreatorProvider could be renamed to TestCreator (that is why I proposed the registration annotation to be in TestCreator outer class). Current state is not logical. You can either really rename TestCreatorProvider to TestCreator, or please move the @Registration annotation into TestCreatorProvider. I don't see the annotation processor in the above change. You need to write one and remove @ServiceProvider(service=TestCreatorProvider.class, position=10), otherwise you want prevent JUnitTestCreatorProvider class to be loaded until really needed. Y04 Don't forget to remove TestCreatorProvider.getDisplayName() it is going to be substituted by the annotation.
(In reply to comment #13) > (In reply to comment #12) > > Fixed [Y01], [Y02] and [Y03]: http://hg.netbeans.org/main/rev/6cb9202dafdc > > Opps, I silently assumed that TestCreatorProvider could be renamed to > TestCreator (that is why I proposed the registration annotation to be in > TestCreator outer class). Current state is not logical. You can either really > rename TestCreatorProvider to TestCreator, or please move the @Registration > annotation into TestCreatorProvider. > > I don't see the annotation processor in the above change. You need to write one > and remove @ServiceProvider(service=TestCreatorProvider.class, position=10), > otherwise you want prevent JUnitTestCreatorProvider class to be loaded until > really needed. > > Y04 Don't forget to remove TestCreatorProvider.getDisplayName() it is going to > be substituted by the annotation. Moved the @Registration annotation into TestCreatorProvider, added TestCreatorProviderProcessor and removed TestCreatorProvider.getDisplayName() method : http://hg.netbeans.org/main/rev/a75fdce41a9a Oh and removed TestCreator: http://hg.netbeans.org/main/rev/995331bfd42f
(In reply to comment #11) > (In reply to comment #9) > > Re. JG04 - [..for] rewriting some of the NetBeans > > tests to TestNG [...] it will be beneficial to have separate module > > libs.testng > Re. JG04: created libs.testng: http://hg.netbeans.org/main/rev/0d92ff204660 > Beware that there is compatibility code in the harness which adds a libs.junit4 > dep if none is given explicitly; this would need to be amended to check for > libs.testng too. I haven't done this yet as I think this should be done after Jarda's experiments. Meanwhile one can use explicit dependency. Or do you prefer me to add the check now?
(In reply to comment #6) > [JG04] ...moving the addLibrary method to > AntTestNGSupport, and making MavenTestNGSupport.configureProject directly add > the desired POM dependency. done: http://hg.netbeans.org/main/rev/e8eed24a1ab7
If there are no more comments I plan to integrate tomorrow. Thanks.
Responded offline to JG03: > I will move TestCreatorAction, TestMethodDebuggerAction and > TestMethodRunnerAction from org.netbeans.modules.gsf.testrunner.api to > org.netbeans.modules.gsf.testrunner (In reply to comment #15) >> there is compatibility code in the harness which adds a libs.junit4 >> dep if none is given explicitly; this would need to be amended to check for >> libs.testng too. > > I haven't done this yet as I think this should be done after Jarda's > experiments. Meanwhile one can use explicit dependency. Not sure you understood what I was saying. The harness checks for a list of test dependencies, which are normally completely enumerated; but for compatibility of old modules set up for tests in the XTest era, when JUnit and NB-JUnit deps were implied, if there is _no_ dep on libs.junit4 then it issues a warning but adds in libs.junit4 and nbjunit. In order to use libs.testng for testing modules, therefore, either this compatibility check in the harness has to be relaxed to treat a dep on libs.testng as indicating that the compat mode is not need; or those modules need to have test deps on _both_ libs.testng and libs.junit4, even though the latter is unused. Branch diff seems to be contaminated with the patch for bug #209058. If you need to maintain multiple interdependent branches, you probably want to be using the pbranch extension.
Lots of lines in diff have trailing whitespace in it. Make sure you configure the NB editor to remove trailing WS from modified lines. [JG05] TestCreatorProviderProcessor has several mistakes: e.getAnnotation(Registration.class) may be null in erroneous code; continue the loop in that case. Do not use .file(String) and try to construct a particular *.instance file name. Rather use instanceFile, which creates the right name (and, if necessary, instanceCreate attribute) automatically (even for ElementKind.METHOD if your annotation allows it), as well as checking assignability of the registered object. Since you are registering under the Services folder, you must specify .stringvalue("instanceOf", TestCreatorProvider.class.getName()). Otherwise every call to Lookup.getDefault().lookup(Anything.class) will load all TCP instances only to find out (using Class.isInstance) that they are irrelevant! On that last topic, [JG06] from looking at the code it seems that Y01 was not satisfied at all - you just call Lookup.getDefault().lookupAll(TCP.class), loading all instances unconditionally. The usage in enable(...) seems like it really needs all of them, since you are calling a method on each; if this is true Y01 is invalid and should be reverted (@ServiceProvider suffices). The usage in performAction(...) is silly: you are loading all of them, _then_ looking up the @Registration after it is too late. (Tip: if you are using RetentionPolicy.RUNTIME on a layer-generating annotation you are generally doing something wrong.) Probably what you meant to do is use lookupResult(TCP.class).allItems() and then using Item.getDisplayName() on each. But this is also calling TCP.canHandleMultipleClasses on each, meaning again the whole attempt at lazy loading is doomed to failure. Since there are only expected to be two implementations of TCP (right?), trying to delay their loading is probably a waste of effort. [JG07] In build-impl, avoid using <isset> in conditions and if="propname" in targets. Since Ant 1.8.0, you can and should use boolean values (true/false) rather than existence or nonexistence of an attribute as a test: so <istrue value="${propname}"/> and <target ... if="${propname}">. The advantage is that you can use -Dpropname=false to disable the condition, as you would intuitively expect; otherwise -Dpropname=<anything> (even =false!) enables it. You can also simplify stuff like <or><not><isset property="use.testng"/></not><isfalse value="${use.testng}"/></or> to just <isfalse value="${use.testng}"/>, or <and><isset property="junit+testng.available"/><isset property="use.testng"/><istrue value="${use.testng}"/></and> to just <and><istrue property="${junit+testng.available}"/><istrue value="${use.testng}"/></and>.
(In reply to comment #19) > Since there are only expected to be two implementations of TCP (right?), trying > to delay their loading is probably a waste of effort. ok I will revert to using the @ServiceProvider annotation. Unless Jardo has another comment on that.
> (In reply to comment #15) > >> there is compatibility code in the harness which adds a libs.junit4 > >> dep if none is given explicitly; this would need to be amended to check for > >> libs.testng too. > > > > I haven't done this yet as I think this should be done after Jarda's > > experiments. Meanwhile one can use explicit dependency. > > [...] In order to use libs.testng for > testing modules, therefore, either this [libs.junit4 and nbjunit] compatibility check in the harness has > to be relaxed to treat a dep on libs.testng as indicating that the compat mode > is not need; Now I understand - http://hg.netbeans.org/main/rev/21999677ab8c > or those modules need to have test deps on _both_ libs.testng and > libs.junit4, even though the latter is unused. It would be hard to find the right test runner to use if both libs are present (in terms of backward compatibility - junit uses formode=pertest while testng uses forkmode=once). So if libs.testng is found (must be explicitly added to module's test deps), then TestNG is used to run tests (in mixed mode, so JUnit tests, if found do run too) I also switched o.n.m.testng module to use TestNG in tests.
(In reply to comment #20) > (In reply to comment #19) > > Since there are only expected to be two implementations of TCP (right?), trying > > to delay their loading is probably a waste of effort. > > ok I will revert to using the @ServiceProvider annotation. Unless Jardo has > another comment on that. I continue to prefer proper declarative registration to @ServiceProvider.
Created attachment 116859 [details] patch for JG05 and JG06 comments ok then. This patch solves the issues mentioned in comment JG05 and JG06.
(In reply to comment #21) > http://hg.netbeans.org/main/rev/21999677ab8c Why the gratuitous reindentation of unrelated lines? Check diffs before commit. This kind of change makes diffs very hard to review, breaks hg annotate, and makes merging long-lived branches more dangerous. (In reply to comment #23) > This patch solves the issues mentioned in comment JG05 and JG06. This code block: for (Lookup.Item<TestCreatorProvider> provider : providers) { return provider.getInstance().enable(activatedNodes); } would seem to just ignore all but the "first" one. Fixing that to logical-or the enable methods of each provider would just make it not be lazy, as JG06 warned. (BTW this part of the patch may have no impact on behavior, since AFAIK lookupAll already returns a lazy collection.)
(In reply to comment #24) > This code block: > > for (Lookup.Item<TestCreatorProvider> provider : providers) { > return provider.getInstance().enable(activatedNodes); > } > > would seem to just ignore all but the "first" one. Fixing that to logical-or > the enable methods of each provider would just make it not be lazy, as JG06 > warned. > > (BTW this part of the patch may have no impact on behavior, since AFAIK > lookupAll already returns a lazy collection.) I need only one provider to return "true". In the worst case I would need to invoke provider.getInstance().enable(activatedNodes); in all of them. The code block should be something like: boolean enable; for (Lookup.Item<TestCreatorProvider> provider : providers) { enable = provider.getInstance().enable(activatedNodes); if(enable) { return true; } } I will change it. Thank you
Created attachment 116928 [details] libs.testng in harness and testdist (In reply to comment #24) > (In reply to comment #21) > > http://hg.netbeans.org/main/rev/21999677ab8c > > Why the gratuitous reindentation of unrelated lines? Check diffs before commit. I'm really sorry for this, it wasn't intentional (I switched to more recent IDE build and forgot to uncheck the only checked 'Ignore' checkbox in options :-( ). Attached the 'hg diff -w' version of the patch. > This kind of change makes diffs very hard to review, breaks hg annotate, and > makes merging long-lived branches more dangerous. Completely agree.
(In reply to comment #25) Changed in branch: http://hg.netbeans.org/main/rev/2f9927070165 If there are no more comments I would like to integrate tomorrow. Thank you
Integrated into 'main-golden', will be available in build *201203230400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/69f5314b6791 User: Lukas Jungmann <jungi@netbeans.org> Log: #208841: [JG01] remove contrib from package name
*** Bug 208105 has been marked as a duplicate of this bug. ***
*** Bug 198564 has been marked as a duplicate of this bug. ***