Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 327681 - "open source" contextual menu no longer appear
Summary: "open source" contextual menu no longer appear
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: TPTP (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Bozier jerome CLA
QA Contact: Kathy Chan CLA
URL:
Whiteboard:
Keywords:
: 324658 (view as bug list)
Depends on:
Blocks: 324658
  Show dependency tree
 
Reported: 2010-10-13 11:32 EDT by Bozier jerome CLA
Modified: 2016-05-05 11:06 EDT (History)
1 user (show)

See Also:
paulslau: review+
paulslau: review+


Attachments
patch (1.12 KB, patch)
2010-10-19 06:36 EDT, Bozier jerome CLA
no flags Details | Diff
Patch V2 (27.76 KB, patch)
2010-10-25 07:25 EDT, Bozier jerome CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bozier jerome CLA 2010-10-13 11:32:42 EDT
switch to test perspective
create a junit plugin test and generate its java source code
select it and right click : there is no "open source" contextual menu

this is a regression, it was working on previous version
after some investigation (old eclipse + old tptp), problem appear if you replace the plugin "org.eclipse.tptp.test.tools.junit.plugin" with the latest one
Comment 1 Bozier jerome CLA 2010-10-13 11:34:29 EDT
unable to fix 324658 with this bug active (we can not verify that the open source command still work after the fix)
Comment 2 Bozier jerome CLA 2010-10-19 06:36:41 EDT
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
Comment 3 Bozier jerome CLA 2010-10-19 07:31:39 EDT
test case added under CVS

Paul, could you review this small patch please ?

many thanks in advance
Comment 4 Paul Slauenwhite CLA 2010-10-19 09:16:20 EDT
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?
Comment 5 Bozier jerome CLA 2010-10-19 11:22:41 EDT
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 ?
Comment 6 Paul Slauenwhite CLA 2010-10-19 11:43:27 EDT
(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!
Comment 7 Bozier jerome CLA 2010-10-20 04:23:40 EDT
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
Comment 8 Paul Slauenwhite CLA 2010-10-20 07:31:32 EDT
(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?
Comment 9 Bozier jerome CLA 2010-10-25 07:25:05 EDT
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
Comment 10 Bozier jerome CLA 2010-10-25 07:30:45 EDT
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
Comment 11 Bozier jerome CLA 2010-10-25 07:32:52 EDT
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
Comment 12 Bozier jerome CLA 2010-10-25 07:34:41 EDT
*** Bug 324658 has been marked as a duplicate of this bug. ***
Comment 13 Paul Slauenwhite CLA 2010-10-25 08:02:34 EDT
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!
Comment 14 Bozier jerome CLA 2010-10-25 09:44:32 EDT
test suite updated to cover 324658
patch fixed following review comment and pushed under CVS
Comment 15 Kathy Chan CLA 2011-02-11 13:46:11 EST
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
Comment 16 Bozier jerome CLA 2011-04-04 09:32:33 EDT
closing