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

Denis Kononenko denis.kononenko at oracle.com
Tue Nov 15 15:16:23 UTC 2016


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



More information about the core-libs-dev mailing list