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

[Maurizio Cimadamore]

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