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

[Thomas Stüfe]

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> 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> 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> 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
>