[9] RFR: 8167240: Write new tests to cover functionality of existing 'jimage' options

[Jim Laskey (Oracle)]

+1

Really nice, thank you.

> On Nov 15, 2016, at 11:16 AM, Denis Kononenko <denis.kononenko at oracle.com> wrote:
> 
> 
> Hi,
> 
> Please do re-review for these changes.
> 
> 1) tests for list --include were rewritten accordingly to https://bugs.openjdk.java.net/browse/JDK-8167384;
> 2) removed tests for '@filename', see https://bugs.openjdk.java.net/browse/JDK-8169720;
> 3) two new CRs were submitted: https://bugs.openjdk.java.net/browse/JDK-8169715, https://bugs.openjdk.java.net/browse/JDK-8169713;
> 
> WEBREV: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.04/
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240
> 
> Thank you,
> Denis.
> 
> 
>> Hi Andrey,
>> 
>> No, it isn't. I submitted a new CR:
>> https://bugs.openjdk.java.net/browse/JDK-8167384.
>> 
>> Thank you,
>> Denis.
>> 
>>> Hi,
>>> 
>>> Looks OK.  Is it 100% pass rate?
>>> 
>>> —Andrey
>>>> On 9 Nov 2016, at 20:36, Denis Kononenko
>>> <denis.kononenko at oracle.com> wrote:
>>>> 
>>>> 
>>>> 
>>>> Hi,
>>>> 
>>>> After discussion with Andrey we decided to add more tests for
>> corner
>>> cases.
>>>> The new changes are available by the link below.
>>>> 
>>>> WEBREV: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.03/
>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240
>>>> 
>>>> 
>>>> Thank you,
>>>> Denis.
>>>> 
>>>>> Hi,
>>>>> 
>>>>> Looks OK to me.
>>>>> I can suggest two more cases. A directory and file symlink can
>> be
>>>>> passed in options where tool requires a file path. 
>>>>> 
>>>>> —Andrey
>>>>> 
>>>>>> On 8 Nov 2016, at 16:17, Denis Kononenko
>>>>> <denis.kononenko at oracle.com> wrote:
>>>>>> 
>>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> The new version of changes.
>>>>>> 
>>>>>> - Switched back to jdk/test/testlibrary to avoid unwanted
>>>>> dependencies (JImageToolTest.java);
>>>>>> - Verified tests on smallest possible JDK build.
>>>>>> 
>>>>>> WEBREV:
>> http://cr.openjdk.java.net/~dkononenko/8167240/webrev.02/
>>>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240
>>>>>> 
>>>>>> Thank you,
>>>>>> Denis.
>>>>>> 
>>>>>>> Denis,
>>>>>>> 
>>>>>>> I can see that you have switched to the top level test library
>>>>> with
>>>>>>> this change. With that you are getting more module
>> dependencies
>>>>> than 
>>>>>>> just java.base. First of all, it would probably make sense to
>>>>> build
>>>>>>> only the classes you needed (which would be 
>>>>>>> jdk.test.lib.process.ProcessTools, I assume), but even if you
>>> only
>>>>>>> build that, jdk.test.lib.process.ProcessTools has dependencies
>>>>> outside
>>>>>>> java.base module.
>>>>>>> 
>>>>>>> You either have to declare @modules in your test or go back to
>>> the
>>>>>>> jdk/test/lib/testlibrary. Then, of course, unneeded module
>>>>>>> dependencies are questionable.
>>>>>>> 
>>>>>>> Shura
>>>>>>> 
>>>>>>> 
>>>>>>>> On Nov 3, 2016, at 6:29 AM, Denis Kononenko
>>>>>>> <denis.kononenko at oracle.com> wrote:
>>>>>>>> 
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> I've done some rework accordingly to Alan's and Shura's
>>> comments:
>>>>>>>> 
>>>>>>>> 1) removed overlapped tests from JImageToolTest.java;
>>>>>>>> 
>>>>>>>> 2) added new tests JImageVerifyTest.java for jimage verify;
>>>>>>>> 
>>>>>>>> 3) reorganized jtreg's tags;
>>>>>>>> 
>>>>>>>> The new WEBREV can be found here:
>>>>>>> http://cr.openjdk.java.net/~dkononenko/8167240/webrev.01/
>>>>>>>> 
>>>>>>>> Thank you,
>>>>>>>> Denis.
>>>>>>>> 
>>>>>>>> On 06.10.2016 19:37, Denis Kononenko wrote:
>>>>>>>>> Hi,
>>>>>>>>> 
>>>>>>>>> Could someone please review these new tests for jimage
>>> utility.
>>>>>>>>> 
>>>>>>>>> There're 5 new files containing tests to cover use cases for
>>>>>>> 'info', 'list', 'extract' and other options. No new tests for
>>>>>>> 'verify'.
>>>>>>>>> 
>>>>>>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240
>>>>>>>>> WEBREV:
>>>>> http://cr.openjdk.java.net/~dkononenko/8167240/webrev.00/
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Thank you,
>>>>>>>>> Denis.
>>>>>>>>