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

2016-11-15 Thread Jim Laskey (Oracle)
+1

Really nice, thank you.

> On Nov 15, 2016, at 11:16 AM, Denis Kononenko  
> 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
>>>  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
>  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
>>>  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.
 



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

2016-11-15 Thread Denis Kononenko

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


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

2016-11-10 Thread Denis Kononenko

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


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

2016-11-09 Thread Andrey Nazarov
 Hi,

Looks OK.  Is it 100% pass rate?

—Andrey
> On 9 Nov 2016, at 20:36, Denis Kononenko  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
>>  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
  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.
> 



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

2016-11-09 Thread Denis Kononenko


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


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

2016-11-08 Thread Andrey Nazarov
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  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
>>  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.
>>> 



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

2016-11-08 Thread Denis Kononenko

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


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

2016-11-03 Thread Alexandre (Shura) Iline
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  
> 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.
> 



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

2016-11-03 Thread Jim Laskey (Oracle)
Nice work. +1



> On Nov 3, 2016, at 10:29 AM, Denis Kononenko  
> 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.
> 



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

2016-11-03 Thread Denis Kononenko

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.




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

2016-10-06 Thread Alexandre (Shura) Iline
Hi, Denis,

By an agreement, @modules should go before any action tag. You use it after 
@build.

Shura

> On Oct 6, 2016, at 9:37 AM, 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.



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

2016-10-06 Thread Denis Kononenko

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.