Bugzilla – Full Text Bug Listing
| Summary: | "open source" contextual menu no longer appear | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Bozier jerome <jerome.bozier> | ||||||
| Component: | TPTP | Assignee: | Bozier jerome <jerome.bozier> | ||||||
| Status: | CLOSED FIXED | QA Contact: | Kathy Chan <kathy> | ||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | paulslau | ||||||
| Version: | unspecified | Flags: | paulslau:
review+
paulslau: review+ |
||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 324658 | ||||||||
| Attachments: |
|
||||||||
|
Description
Bozier jerome
unable to fix 324658 with this bug active (we can not verify that the open source command still work after the fix) Created attachment 181167 [details]
patch
This patch fix the problem (thanks Julian for the help)
seems that this condition in plugin.xml was no longer working :
<iterate>
<or>
<instanceof value="org.eclipse.hyades.test.tools.ui.java.internal.junit.navigator.JUnitTestSuiteProxyNode"/>
<instanceof value="org.eclipse.hyades.test.tools.ui.java.internal.junit.navigator.JUnitTestCaseProxyNode"/>
<instanceof value="org.eclipse.hyades.test.tools.ui.http.internal.junit.navigator.HTTPTestSuiteProxyNode"/>
<instanceof value="org.eclipse.hyades.test.tools.ui.http.internal.junit.navigator.HTTPTestCaseProxyNode"/>
</or>
we have completed the check to cover the actual class hierarchy
test case added under CVS Paul, could you review this small patch please ? many thanks in advance Reviewed with comments: -Since we are referencing a class is a dependent plug-in there is a plugin.xml warning. I would suggest trying to implement the command handler in the org.eclipse.tptp.test.tools.junit.plugin plug-in. Wondering if there is an extension mechanism built into the org.eclipse.ui.handlers extension point? after extended testing, some more details about this bug : . menu appear for junit test . menu does not appear for junit plugin test so, i am no longer sure that this bug is really a regression, my problem was that i was always testing with junit plugin test with the previous patch, this contextual menu become also available for junit plugin test (and test case), so it is an improvement now, i understand that putting this patch inside tools.ui is not the best place what we should do : . move OpenSourceCommandHandler to a package that is not flagged as "internal" (org.eclipse.hyades.test.tools.ui.java.junit for example) . register another extension point inside tools.junit.plugin that will use that same OpenSourceCommandHandler (it is why previous point is needed) main advantage would be that junit plugin test are using exactly the same general class, but that extension point registration is splitted (junit test inside tools.ui, junit plugin test inside tools.junit.plugin). on this way, there would no longer be an implicit dependency from tools.ui toward tools.junit.plugin, and potential patch on OpenSouceCommandHandler (c.f. 324658) will apply to both configuration would you agree on such fix ? (In reply to comment #5) > after extended testing, some more details about this bug : > . menu appear for junit test > . menu does not appear for junit plugin test > so, i am no longer sure that this bug is really a regression, my problem was > that i was always testing with junit plugin test > > with the previous patch, this contextual menu become also available for junit > plugin test (and test case), so it is an improvement > > now, i understand that putting this patch inside tools.ui is not the best place > > what we should do : > . move OpenSourceCommandHandler to a package that is not flagged as "internal" > (org.eclipse.hyades.test.tools.ui.java.junit for example) > . register another extension point inside tools.junit.plugin that will use that > same OpenSourceCommandHandler (it is why previous point is needed) > > main advantage would be that junit plugin test are using exactly the same > general class, but that extension point registration is splitted (junit test > inside tools.ui, junit plugin test inside tools.junit.plugin). > on this way, there would no longer be an implicit dependency from tools.ui > toward tools.junit.plugin, and potential patch on OpenSouceCommandHandler (c.f. > 324658) will apply to both configuration > > would you agree on such fix ? Perfect! after lots of investigations last evening, i can't apply directly this solution : first plugin define a "command" inside plugin.xml and we can not reuse it in another plugin to extend the availability of this command. i tryed to define another command that reuse same class behind but i was faced with 2 other problems : there was conflict with key binding (same shortcut for both commands) and i could not reuse displayed string in the contextual menu (this string was defined inside plugin.properties) i am looking on a third solution (defining an extension point in tools.ui plugin to let the user able to add its own class to enable the menu) will try it next week (In reply to comment #7) > after lots of investigations last evening, i can't apply directly this solution > : > first plugin define a "command" inside plugin.xml and we can not reuse it in > another plugin to extend the availability of this command. > i tryed to define another command that reuse same class behind but i was faced > with 2 other problems : there was conflict with key binding (same shortcut for > both commands) and i could not reuse displayed string in the contextual menu > (this string was defined inside plugin.properties) > > i am looking on a third solution (defining an extension point in tools.ui > plugin to let the user able to add its own class to enable the menu) > will try it next week How about defining a 'Java' proxy (in the org.eclipse.hyades.test.ui plug-in) that the JUnit/URL/JUnit Plug-in test suite/case proxies would extend so our command handler would know it has Java source? Created attachment 181630 [details]
Patch V2
this patch replace previous one
now, if a user want to have the "open source" entry available in contextual menu, he have 2 choice :
. general one : implement IJavaSourceTestCaseProxyNode or IJavaSourceTestSuiteProxyNode interfaces. in this case, he have to provide a way to retrieve java file name or method from his proxy, a way to retrieve proxy name in case of error (if file or method does not exist) and a way to unload testSuite (optional, this method can simply do nothing)
. simpler one, directly inherit from "JavaSourceTestCaseProxyNode" or "JavaSourceTestCaseProxyNode". theses two abstract class inherit directly from DefaultTestCaseProxyNode and DefaultTestSuiteProxyNode, and implement the previous interfaces with the needed method
OpenSourceCommandHandler (and plugin.xml) now use theses interface method, and existing proxy nodes (Http, JUnit and JUnit Plugin test suite/case) now inherit from theses abstract classes
this method is a bit more complicated that previous one but on other side, it does not add implicit dependency between plugin, and provide a general way for users to use this menu entry
Jerome
i forgot to add, this patch also fix https://bugs.eclipse.org/bugs/show_bug.cgi?id=324658 now, "open source" contextual menu is always enable if underlying proxy of selected element implement IJavaSourceTestCaseProxyNode or IJavaSourceTestSuiteProxyNode. if user select this contextual menu and file/method does not exists, an error message box open Jerome updating worked hours Paul, could you please review this patch ? I will update test case to cover this fix (check for http test suite/case) + add a performace check for https://bugs.eclipse.org/bugs/show_bug.cgi?id=324658 many thanks in advance, Jerome *** Bug 324658 has been marked as a duplicate of this bug. *** Attached patch (version 2) reviewed with the following comments: -Add the following to the @provisional comments: @provisional As of TPTP V4.7.2 release, this is stable provisional API (see http://www.eclipse.org/tptp/home/documents/process/development/api_contract.html). -Confirm the @version and copyright years are 2010 (see copyJUnitTestSuiteProxyNode and JUnitTestCaseProxyNode). -Open a defect to update the error messages in OpenSourceCommandHandler in TPTP 4.8. -Great job! test suite updated to cover 324658 patch fixed following review comment and pushed under CVS This defect had been resolved as FIXED for more than 1 month. Please verify with the latest TPTP 4.7.2 driver. If this defect is still left unverified by February 25, we'll close it on the originator's behalf. TPTP 4.7.2 driver can be downloaded from: http://www.eclipse.org/tptp/home/downloads/?ver=4.7.2 closing |