Hi Dima, the fix looks good to me,
Cheers, — Igor > On Dec 23, 2016, at 7:13 PM, Dmitry Fazunenenko <dmitry.fazune...@oracle.com> > wrote: > > Hi, > > new version: 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/ >>> >>> -- 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 >>>>> >>>>> 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)---------- >>>>> >>>> >>> >> >
