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

Reply via email to