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

Andrey Nazarov andrey.x.nazarov at oracle.com
Tue Nov 8 14:04:55 UTC 2016


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



More information about the core-libs-dev mailing list