Community
Participate
Working Groups
Build Identifier: M20100211-1343 Using my customized Runner extended from JUnit, when the method has some parameters. It can't be recognized when double clicking on the method name in JUnit View. A message box with "Method 'testXxxx' not found. Opening the test class." will be given. Then in the source code view, the EDE can't locate the right line to the method. Reproducible: Always Steps to Reproduce: 1.RunWith a customized Runner, with the runner the test method may have parameters, and the parameters can be injected by the runner. 2.The case is success or failed. 3.Double clicking the case in the JUnit View. 4.A message box with "Method 'testXxxx' not found. Opening the test class." will be given.
Created attachment 194130 [details] Sceen
Created attachment 194133 [details] A customized JUnitRunner
Created attachment 194136 [details] Sample Test Case
Moving to JDT UI for comment.
OpenTestAction#findElement(IJavaProject, String) could be made more resilient in case no matching method without parameters could be found. A working patch would definitely speed things up ;-)
see also Bug 391023
Created attachment 236753 [details] Patch to solve this issue I have created a patch to solve this issue because we encountered the same problem using our junit-dataprovider (see https://github.com/TNG/junit-dataprovider).
Comment on attachment 236753 [details] Patch to solve this issue The patch goes into the right direction but should be improved - only look for methods that either start with "test" and have no arguments (if JUnit 3 test case) or are annotated with @Test (Junit 4) - for JUnit 4 handle the case where the same method exists with different parameters (ask the user if more than one)
OK, I see the problems. Is there a GIT-URL such that I can fork and send a pull request?
(In reply to Andreas Schmid from comment #9) > OK, I see the problems. > Is there a GIT-URL such that I can fork and send a pull request? We don't pull from other repos, but you can provide a patch or upload the change via Gerrit. You also need to sign the CLA.
To provide a patch, would be easier for me, if I would know the source repository to check out. Currently I used the corresponding Eclipse feature to get it done ...
(In reply to Andreas Schmid from comment #11) > To provide a patch, would be easier for me, if I would know the source > repository to check out. Currently I used the corresponding Eclipse feature > to get it done ... Of course. Here you go: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/
Created attachment 237275 [details] Enhanced patch to fix this bug I have once again tried to create a patch considering comment 8 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=343935#c8). And now I tried to consider also "only look for methods that either start with "test" and have no arguments (if JUnit 3 test case) or are annotated with @Test (Junit 4)". But still unclear for me is the ask user part in "for JUnit 4 handle the case where the same method exists with different parameters (ask the user if more than one)". Maybe you can give me same hints or just fix that your own? My only idea is to copy the lines 117-120 (in OpenTestAction.java) and adjust them properly. But as we return null the origin part in OpenTestAction.java will be executed as well. I am not sure how you usually solve such problems even if I have some ideas ... Maybe you can give me same hints and/or examples to the last point clear for me. I would enjoy to help :)
(In reply to Andreas Schmid from comment #13) > Created attachment 237275 [details] [diff] > Enhanced patch to fix this bug > > I have once again tried to create a patch considering comment 8 > (https://bugs.eclipse.org/bugs/show_bug.cgi?id=343935#c8). > > And now I tried to consider also "only look for methods that either start > with "test" and have no arguments (if JUnit 3 test case) or are annotated > with @Test (Junit 4)". > > But still unclear for me is the ask user part in "for JUnit 4 handle the > case where the same method exists with different parameters (ask the user if > more than one)". What I meant, is to present the list of ambiguous methods to the user, so that he can choose one.
(In reply to Dani Megert from comment #14) > (In reply to Andreas Schmid from comment #13) > > Created attachment 237275 [details] [diff] > > Enhanced patch to fix this bug > > > > I have once again tried to create a patch considering comment 8 > > (https://bugs.eclipse.org/bugs/show_bug.cgi?id=343935#c8). > > > > And now I tried to consider also "only look for methods that either start > > with "test" and have no arguments (if JUnit 3 test case) or are annotated > > with @Test (Junit 4)". > > > > But still unclear for me is the ask user part in "for JUnit 4 handle the > > case where the same method exists with different parameters (ask the user if > > more than one)". > > What I meant, is to present the list of ambiguous methods to the user, so > that he can choose one. Do you have a code example for me? Or can you fix it?
(In reply to Andreas Schmid from comment #15) > > What I meant, is to present the list of ambiguous methods to the user, so > > that he can choose one. > > Do you have a code example for me? Or can you fix it? See SelectionConverter.selectJavaElement(IJavaElement[], Shell, String, String)
Created attachment 237784 [details] Patch to solve this issue (considering all comments) Next try for a patch to this issue, this time with an dedicated chooser dialog if multiple Methods exists.
Created attachment 237785 [details] Patch to solve this issue (considering all comments) Sorry attached wrong file in comment #17. Next try for a patch to this issue, this time with an dedicated chooser dialog if multiple Methods exists.
Thanks again, for merging this issue. Is there already a release date for a new version of eclipse-junit containing this fix?
Comment on attachment 237785 [details] Patch to solve this issue (considering all comments) I get compile errors when I apply this patch.
Which repo and commit do you use?
(In reply to Andreas Schmid from comment #21) > Which repo and commit do you use? http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/tree/org.eclipse.jdt.junit master branch
I checked out http://git.eclipse.org/gitroot/jdt/eclipse.jdt.ui.git and applied the patch. Unfortunately, I cannot build it using "mvn package" (due to "Unable to satisfy dependency from org.eclipse.ltk.core.refactoring 3.6.100.qualifier to bundle org.eclipse.text [3.5.0,4.0.0).; No solution found because the problem is unsatisfiable.]") but in Eclipse it is working! I am sorry to cannot help you further ... Can you nevertheless fix it?
(In reply to Andreas Schmid from comment #23) > I checked out http://git.eclipse.org/gitroot/jdt/eclipse.jdt.ui.git and > applied the patch. Unfortunately, I cannot build it using "mvn package" (due > to "Unable to satisfy dependency from org.eclipse.ltk.core.refactoring > 3.6.100.qualifier to bundle org.eclipse.text [3.5.0,4.0.0).; No solution > found because the problem is unsatisfiable.]") but in Eclipse it is working! > > I am sorry to cannot help you further ... Can you nevertheless fix it? If you look at your patch, you'll see that you use classes which aren't imported. This is doomed to fail. No, I don't have time to work on this, sorry.
Created attachment 239978 [details] other try to patch this issue without compiliing errors (In reply to Dani Megert from comment #24) > If you look at your patch, you'll see that you use classes which aren't > imported. This is doomed to fail. > > No, I don't have time to work on this, sorry. I'm sorry for that, but as I cannot compile using "mvn package" due to "Unable to satisfy dependency from [...]" I am just able to import the plugin within Eclipse and try to apply the changes to source code from git manually. Here's another try! P.S.: Which repository do I have to add to my settings.xml or can you provide me yours, that I am able to compile it using "mvn compile"?
(In reply to Andreas Schmid from comment #25) > Here's another try! Thanks. This patch now compiles! Using internal SelectionConverter is a corner case and OK in this case, but you should never reuse internal messages from another bundle. They should be copied into JUnitMessages. Please also sign the CLA. Without that we can't take your patch. > P.S.: Which repository do I have to add to my settings.xml or can you > provide me yours, that I am able to compile it using "mvn compile"? You should use the Eclipse IDE + EGit and not mvn or other tools, unless of course *you* know how to compile there ;-). In Eclipse with EGit support you can simply clone the git://git.eclipse.org/gitroot/jdt/eclipse.jdt.ui.git repository and import the JUnit project from that cloned repository.
Created attachment 241439 [details] fixed bug 343935 (In reply to Dani Megert from comment #26) > > Thanks. This patch now compiles! Using internal SelectionConverter is a > corner case and OK in this case, but you should never reuse internal > messages from another bundle. They should be copied into JUnitMessages. done, see new patch. > > Please also sign the CLA. Without that we can't take your patch. done, see https://projects.eclipse.org/users/andreas-schmid/cla > > You should use the Eclipse IDE + EGit and not mvn or other tools, unless of > course *you* know how to compile there ;-). In Eclipse with EGit support you > can simply clone the git://git.eclipse.org/gitroot/jdt/eclipse.jdt.ui.git > repository and import the JUnit project from that cloned repository. Thanks, I have tried that and clone as well as import works properly. Unfortunately, the imported projects from master branch shows round about 3000 problems in Eclipse problem view ...
(In reply to Dani Megert from comment #26) > (In reply to Andreas Schmid from comment #25) > > Here's another try! > > but you should never reuse internal > messages from another bundle. They should be copied into JUnitMessages. This is still the case in the latest patch.
But I have added and reference following two new entries in ActionMessages.java (copied from attachment "fixed bug 343935"): + public static String OpenTestAction_description; + public static String OpenTestAction_select_element;
(In reply to Andreas Schmid from comment #29) > But I have added and reference following two new entries in > ActionMessages.java (copied from attachment "fixed bug 343935"): > > + public static String OpenTestAction_description; > + public static String OpenTestAction_select_element; But in the wrong plug-in. Messages should never be accessed from another bundle. Hence also this is not correct: +import org.eclipse.debug.internal.ui.actions.ActionMessages;
Created attachment 242044 [details] fixed bug 343935 (In reply to Dani Megert from comment #30) > But in the wrong plug-in. Messages should never be accessed from another > bundle. > > Hence also this is not correct: > +import org.eclipse.debug.internal.ui.actions.ActionMessages; I am sorry. I just feel to be too stupid to get it right. Maybe this time? P.S. Thanks for being that patient with me ;-)
Created attachment 242051 [details] Picture showing conflict This patch does not apply on 'master'.
Created attachment 242054 [details] fixed bug 343935 (eclipse patch) (In reply to Dani Megert from comment #32) > Created attachment 242051 [details] > Picture showing conflict > > This patch does not apply on 'master'. arg ... you want to apply it using Eclipse ... please use the new attachment. (With "git apply $FILE" it should have worked either ...)
The patch looks good. Just some minor tweaks: > OpenTestAction_description This should be OpenTestAction_title with value "Go to Test" When I hit cancel on the dialog, I get another dialog. That's not OK. It should just do nothing in that case. Please also add your credentials to the copyright notices, e.g. Terry Parker <tparker@google.com> - Protect against poorly behaved completion proposers - http://bugs.eclipse.org/429925 You need to confirm that you want to contribute this by saying: "This contribution complies with http://www.eclipse.org/legal/CoO.php" Thanks.
Created attachment 242074 [details] fixed bug 343935 (eclipse patch) Hi Dani, (In reply to Dani Megert from comment #34) > The patch looks good. Just some minor tweaks: thanks so far for your effort ... > > > OpenTestAction_description > This should be OpenTestAction_title with value "Go to Test" done. > > When I hit cancel on the dialog, I get another dialog. That's not OK. It > should just do nothing in that case. Ah ok, I see, this is really a design flaw. I know returned the list of found element (never null) and handled this list in the calling method properly. I like this even better because the showing of dialogs is handling within the same method. Now I open the "type" if the user clicks "Cancel" (same as with error if no method is found). Unfortunately, I cannot do anything because returning a "null" leads to a "MessageDialog.openError(getShell(), ...);" in the calling method "org.eclipse.jdt.internal.junit.ui.OpenEditorAction.run()" What do you think? > > Please also add your credentials to the copyright notices, e.g. > Terry Parker <tparker@google.com> - Protect against poorly behaved > completion proposers - http://bugs.eclipse.org/429925 done. > > You need to confirm that you want to contribute this by saying: > > "This contribution complies with http://www.eclipse.org/legal/CoO.php" I confirm, that this contribution complies with http://www.eclipse.org/legal/CoO.php. > > Thanks. Attached is the new patch (unfortunately I couldn't directly pushed it to gerrit (ref/for/master) :-( (as I say, Terry did that on the linked issue)
I agree fixing the second dialog issue is a but strange. Instead of polluting the code for that corner case I think we live with it. I've combined your last two patches into one, fixed the NLS warnings and committed as http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=f6db2c136a709858311cfe3a3a7938e8947b9e57 Thanks Andreas! > Attached is the new patch (unfortunately I couldn't directly pushed it to > gerrit (ref/for/master) :-( (as I say, Terry did that on the linked issue) You need to configure your repository for Gerrit. For details see https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Provide_a_contribution_using_Gerrit
(In reply to Dani Megert from comment #36) > I agree fixing the second dialog issue is a but strange. Instead of > polluting the code for that corner case I think we live with it. > > I've combined your last two patches into one, fixed the NLS warnings and > committed as > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=f6db2c136a709858311cfe3a3a7938e8947b9e57 great! > > Thanks Andreas! No, thank you for your help! > > > > Attached is the new patch (unfortunately I couldn't directly pushed it to > > gerrit (ref/for/master) :-( (as I say, Terry did that on the linked issue) > > You need to configure your repository for Gerrit. For details see > https://wiki.eclipse.org/JDT_UI/ > How_to_Contribute#Provide_a_contribution_using_Gerrit Will do it for the next bug :)
