RFR: CODETOOLS-7902373: Intellij jtreg plugin fails to resolve external libraries

[Maurizio Cimadamore]

Approved and pushed.

Thanks
Maurizio

On 31/01/2019 14:56, Thomas Stüfe wrote:
> Great, here you go:
>
> http://cr.openjdk.java.net/~stuefe/webrevs/codetools-7902373--intellij-jtreg-plugin-fails-to-resolve-external-libraries/webrev.01/webrev/
>
> Only minor stuff changed:
>
> - added change note as you suggested (found that <ul> is needed to 
> make it look good in the settings dialog though)
> - bumped version to 1.9
> - corrected copyright dates
>
> Thank you for pushing! My census name is "stuefe", if that is important.
>
> Cheers, Thomas
>
>
>
>
> On Thu, Jan 31, 2019 at 3:30 PM Maurizio Cimadamore 
> <maurizio.cimadamore at oracle.com 
> <mailto:maurizio.cimadamore at oracle.com>> wrote:
>
>     I'm happy to push it for you when you have the final changeset.
>
>     Thanks!
>     Maurizio
>
>     On 30/01/2019 18:17, Jonathan Gibbons wrote:
>     > If Maurizio approves, you're good to go, although you're not a
>     > CodeTools committer, and will need someone (e.g. him or me) to push
>     > for you.
>     >
>     > -- Jon
>     >
>     > On 1/30/19 9:24 AM, Thomas Stüfe wrote:
>     >> Thanks Maurizio. I'll prepare another webrev, but probably not
>     before
>     >> next
>     >> week.
>     >>
>     >> Do I need a second reviewer for codetool patches?
>     >>
>     >> Cheers, Thomas
>     >>
>     >> On Wed, Jan 30, 2019 at 5:27 PM Maurizio Cimadamore <
>     >> maurizio.cimadamore at oracle.com
>     <mailto:maurizio.cimadamore at oracle.com>> wrote:
>     >>
>     >>> The patch seems to be working correctly - I used it in my
>     existing
>     >>> project
>     >>> and it seems to work fine.
>     >>>
>     >>> As for the review, you need to bump the jtreg plugin version,
>     e.g. like
>     >>> this:
>     >>>
>     >>> --- a/plugins/idea/resources/META-INF/plugin.xml        Tue Jan 29
>     >>> 15:11:03 2019 -0800
>     >>> +++ b/plugins/idea/resources/META-INF/plugin.xml        Wed Jan 30
>     >>> 16:27:18 2019 +0000
>     >>> @@ -26,13 +26,13 @@
>     >>>   <idea-plugin>
>     >>>       <id>jtreg</id>
>     >>>       <name>jtreg Test Support</name>
>     >>> -    <version>1.8</version>
>     >>> +    <version>1.9</version>
>     >>>       <description><![CDATA[
>     >>>         Allows execution of tests developed using the <a href=
>     >>> "http://openjdk.java.net/jtreg/"
>     >>> <http://openjdk.java.net/jtreg/>>jtreg</a>
>     >>> framework.
>     >>>       ]]></description>
>     >>>
>     >>>       <change-notes><![CDATA[
>     >>> -      Add support for IntelliJ IDE2018.3
>     >>> +      Allow support for external library resolution
>     >>>       ]]>
>     >>>       </change-notes>
>     >>>
>     >>>
>     >>> Thanks
>     >>> Maurizio
>     >>> On 29/01/2019 14:32, Thomas Stüfe wrote:
>     >>>
>     >>>
>     >>> On Tue, Jan 29, 2019 at 3:24 PM Maurizio Cimadamore <
>     >>> maurizio.cimadamore at oracle.com
>     <mailto:maurizio.cimadamore at oracle.com>> wrote:
>     >>>
>     >>>> Hi,
>     >>>> this looks like a solid piece of work; thanks for adding all the
>     >>>> documents, they make what's going on very explicit.
>     >>>>
>     >>>>
>     >>> Thanks!
>     >>>
>     >>>
>     >>>> Do you have a pointer to a test that rely on this feature?
>     I'd like to
>     >>>> give this patch a spin.
>     >>>>
>     >>> For example, anything in hotspot/jtreg/runtime, e.g.
>     >>> runtime/ErrorHandling. Alternatively, you could grep for "import
>     >>> jdk.test.lib.process". There are some tests in the jdk too.
>     >>>
>     >>> Cheers, Thomas
>     >>>
>     >>>
>     >>>> Cheers
>     >>>> Maurizio
>     >>>>
>     >>>> On 29/01/2019 14:06, Thomas Stüfe wrote:
>     >>>>> Greetings,
>     >>>>>
>     >>>>> May I please have your opinions about the following fix:
>     >>>>>
>     >>>>> bug: https://bugs.openjdk.java.net/browse/CODETOOLS-7902373
>     >>>>> cr:
>     >>>>>
>     >>>>
>     http://cr.openjdk.java.net/~stuefe/webrevs/codetools-7902373--intellij-jtreg-plugin-fails-to-resolve-external-libraries/webrev.00/webrev/
>
>     >>>>
>     >>>>> jtreg tests containing references to external test libraries
>     (e.g.
>     >>>> @library
>     >>>>> /test/lib) are not properly handled(e.g.
>     jdk.test.lib.process.*), and
>     >>>>> references to those test support classes will not be
>     resolved and
>     >>>> result in
>     >>>>> red wiggly lines in Intellij. This affects mostly jtreg
>     tests for the
>     >>>>> hotspot, since those typically rely on /test/lib.
>     >>>>>
>     >>>>> This patch causes the Intellij jtreg plugin properly
>     recognize and
>     >>>> handle
>     >>>>> the external.lib.roots entry in the test suite configuration
>     >>>>> files. The
>     >>>>> TEST.ROOT file for an opened source is parsed and the external
>     >>>>> library
>     >>>> path
>     >>>>> extracted. I am not sure whether this is the best way to go
>     about
>     >>>>> it or
>     >>>>> whether this is too expensive though. The parsing is only
>     done for
>     >>>>> the
>     >>>>> first file under a given TEST.ROOT, so it should be okay?
>     >>>>>
>     >>>>> The plugin works for me, on a quite underpowered ultrabook.
>     Note
>     >>>>> that I
>     >>>>> also added a lot of logging, all debug level, to aid me in
>     >>>>> development.
>     >>>> If
>     >>>>> necessary I can remove it but would prefer not to.
>     >>>>>
>     >>>>> Please note that I'm quite new to jtreg plugin development
>     as well
>     >>>>> as to
>     >>>>> the jtreg internals, so I may miss something obvious. Would
>     >>>>> appreciate
>     >>>> it
>     >>>>> if someone more knowledgeable reviews it (Maurizio?).
>     >>>>>
>     >>>>> Thanks & Best Regards, Thomas
>