Hi, 2 minor comments. bug id has not been added “@bug 8171441” javadoc should start with /** not 152 /*
—Andrey > On 26 Dec 2016, at 17:40, Stanislav Smirnov <stanislav.smir...@oracle.com> > wrote: > > Hi, > > thanks, looks good > > Best regards, > Stanislav Smirnov > > > > > >> On 23 Dec 2016, at 19:13, Dmitry Fazunenenko <dmitry.fazune...@oracle.com> >> wrote: >> >> Hi, >> >> new version: http://cr.openjdk.java.net/~dfazunen/8171441/webrev.02/ >> <http://cr.openjdk.java.net/~dfazunen/8171441/webrev.02/> >> >> This comment now looks like: >> 152 /* >> 153 * Checks if the tools accept "-version" option (exit code is zero). >> 154 * The output of the tools run with "-version" is not verified. >> 155 */ >> >> Thanks >> Dima >> >> On 23.12.2016 13:40, Stanislav Smirnov wrote: >>> Hi, >>> >>> sorry, missed this strange comment during the first review. >>> 152 /* >>> 153 * this tests if the tool can take a version string and returns >>> 154 * a 0 exit code, it is not possible to validate the contents >>> 155 * of the -version output as they are inconsistent. >>> 156 */ >>> 157 static String testToolVersion() { >>> It confuses me, can you please rephrase it? >>> >>> Best regards, >>> Stanislav Smirnov >>> >>> >>> >>> >>> >>>> On 23 Dec 2016, at 11:55, Dmitry Fazunenko <dmitry.fazune...@oracle.com >>>> <mailto:dmitry.fazune...@oracle.com>> wrote: >>>> >>>> Hi Stanislav, >>>> >>>> Thanks for looking. >>>> Updated webrev: >>>> http://cr.openjdk.java.net/~dfazunen/8171441/webrev.01/ >>>> <http://cr.openjdk.java.net/%7Edfazunen/8171441/webrev.01/> >>>> >>>> -- Dima >>>> >>>> >>>> On 22.12.2016 19:55, Stanislav Smirnov wrote: >>>>> Hi Dima, >>>>> >>>>> changes look good, I will only suggest using lambda’s in couple of >>>>> iterations >>>>> + for (String s : tr.testOutput) { >>>>> + System.out.println(s); >>>>> + } >>>>> tr.testOutput.forEach(System.out::println) >>>>> >>>>> for (String x : tr.testOutput) { >>>>> alist.add(x.trim()); >>>>> } >>>>> tr.testOutput.stream.map(String::trim).forEach(aList:add) >>>>> >>>>> >>>>> Best regards, >>>>> Stanislav Smirnov >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>> On 21 Dec 2016, at 17:16, Dmitry Fazunenenko >>>>>> <dmitry.fazune...@oracle.com <mailto:dmitry.fazune...@oracle.com>> wrote: >>>>>> >>>>>> Hello, >>>>>> >>>>>> I'm looking for reviews of a relatively simple test change: >>>>>> http://cr.openjdk.java.net/~dfazunen/8171441/webrev.00/ >>>>>> <http://cr.openjdk.java.net/%7Edfazunen/8171441/webrev.00/> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8171441 >>>>>> <https://bugs.openjdk.java.net/browse/JDK-8171441> >>>>>> >>>>>> The purpose of the change is to improve diagnostic. >>>>>> >>>>>> Thanks, >>>>>> Dima >>>>>> >>>>>> PS: After the fix the failures will be reported as: >>>>>> >>>>>> ----------System.err:(13/956)---------- >>>>>> java.lang.AssertionError: VersionCheck failed: testJVersionStrings: >>>>>> [java]; testToolVersion: [jar]; >>>>>> at VersionCheck.main(VersionCheck.java:295) >>>>>> at >>>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >>>>>> Method) >>>>>> at >>>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) >>>>>> at >>>>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >>>>>> at java.base/java.lang.reflect.Method.invoke(Method.java:538) >>>>>> at >>>>>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110) >>>>>> at java.base/java.lang.Thread.run(Thread.java:844) >>>>>> >>>>>> JavaTest Message: Test threw exception: java.lang.AssertionError: >>>>>> VersionCheck failed: testJVersionStrings: [java]; testToolVersion: [jar]; >>>>>> JavaTest Message: shutting down test >>>>>> >>>>>> STATUS:Failed.`main' threw exception: java.lang.AssertionError: >>>>>> VersionCheck failed: testJVersionStrings: [java]; testToolVersion: [jar]; >>>>>> >>>>>> PPS: The output of passed test now looks like: >>>>>> >>>>>> === testJVersionStrings === >>>>>> Testing servertool >>>>>> Testing jstat >>>>>> Testing jmod >>>>>> Testing jjs >>>>>> Testing jimage >>>>>> Testing jlink >>>>>> Testing jrunscript >>>>>> Testing jdeprscan >>>>>> Testing jconsole >>>>>> Testing rmiregistry >>>>>> Testing keytool >>>>>> Testing schemagen >>>>>> Testing javac >>>>>> Testing jar >>>>>> Testing jhsdb >>>>>> Testing jcmd >>>>>> Testing jstack >>>>>> Testing wsgen >>>>>> Testing jshell >>>>>> Testing serialver >>>>>> Testing jmap >>>>>> Testing javap >>>>>> Testing jps >>>>>> Testing jstatd >>>>>> Testing javadoc >>>>>> Testing tnameserv >>>>>> Testing jdb >>>>>> Testing jinfo >>>>>> Testing jdeps >>>>>> Testing xjc >>>>>> Testing rmid >>>>>> Testing jarsigner >>>>>> Testing idlj >>>>>> Testing rmic >>>>>> Testing appletviewer >>>>>> Testing pack200 >>>>>> Testing javah >>>>>> Testing policytool >>>>>> Testing orbd >>>>>> testJVersionStrings passed >>>>>> === testInternalStrings === >>>>>> testInternalStrings passed >>>>>> === testToolVersion === >>>>>> Testing java >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/java >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/java> >>>>>> -version >>>>>> java version "9-ea" >>>>>> Java(TM) SE Runtime Environment (build 9-ea+149) >>>>>> Java HotSpot(TM) 64-Bit Server VM (build 9-ea+149, mixed mode) >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing javac >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/javac >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/javac> >>>>>> -version >>>>>> javac 9-ea >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing jhsdb >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/jhsdb >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/jhsdb> >>>>>> -version >>>>>> clhsdb command line debugger >>>>>> debugd debug server >>>>>> hsdb ui debugger >>>>>> jstack --help to get more information >>>>>> jmap --help to get more information >>>>>> jinfo --help to get more information >>>>>> jsnap --help to get more information >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing jshell >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/jshell >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/jshell> >>>>>> -version >>>>>> jshell 9-ea >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing javap >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/javap >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/javap> >>>>>> -version >>>>>> 9-ea >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing jdb >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/jdb >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/jdb> >>>>>> -version >>>>>> This is jdb version 9.0 (Java SE version 9-ea) >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing idlj >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/idlj >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/idlj> >>>>>> -version >>>>>> IDL-to-Java compiler (portable), version "3.2" >>>>>> #> echo $? >>>>>> 0 >>>>>> Testing javah >>>>>> #> >>>>>> /net/jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/javah >>>>>> >>>>>> <http://jse-st01.ru.oracle.com/export4/java/re/jdk/9/promoted/ea/149/binaries/solaris-x64/bin/javah> >>>>>> -version >>>>>> >>>>>> Warning: The javah tool is planned to be removed in the next major >>>>>> JDK release. The tool has been superseded by the '-h' option added >>>>>> to javac in JDK 8. Users are recommended to migrate to using the >>>>>> javac '-h' option; see the javac man page for more information. >>>>>> >>>>>> javah version "9-ea" >>>>>> #> echo $? >>>>>> 0 >>>>>> testToolVersion passed >>>>>> === testInternalStrings === >>>>>> testDebugVersion passed >>>>>> All Version string comparisons: PASS >>>>>> ----------System.err:(1/15)---------- >>>>>> >>>>> >>>> >>> >> >