RFR 8171441: tools/launcher/VersionCheck.java doesn't report names of tools which failed checks

Dmitry Fazunenenko dmitry.fazunenko at oracle.com
Mon Dec 26 16:00:44 UTC 2016 


On 26.12.2016 18:45, Andrey Nazarov wrote:
>> On 26 Dec 2016, at 18:36, Dmitry Fazunenenko <dmitry.fazunenko at oracle.com> wrote:
>>
>> Hi Andrey,
>>
>> On 26.12.2016 17:53, Andrey Nazarov wrote:
>>> Hi,
>>>
>>> 2 minor comments.
>>> bug id  has not been added “@bug 8171441”
>> I believe the purpose of the @bug tag is to list the issues the test verifies or the test is regression test for.
>> And it's not need to mention all the problem with the test itself here.
> My understanding is that @bug tag is used to track the reasons for the test modifications and creation. But I haven’t seen any documentation for it.

It's not needed to list CRs related to the test modification, they could 
be extracted from the history.
Much more useful is to list the regressions caught by the test.

-- Dima


>>> javadoc should start with /** not  152 /*
>> I intentionally kept "/*", because it's not a javadoc comment, but just a comment to a method.
>> If we convert this to javadoc, we need to provide javadoc to other similar methods.
>> Of course this would be good, but I'm afraid this activity is rather a separate RFE.
> Ok.
>> Thanks,
>> Dima
>>
>>>
>>> —Andrey
>>>> On 26 Dec 2016, at 17:40, Stanislav Smirnov <stanislav.smirnov at oracle.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> thanks, looks good
>>>>
>>>> Best regards,
>>>> Stanislav Smirnov
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> On 23 Dec 2016, at 19:13, Dmitry Fazunenenko <dmitry.fazunenko at 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.fazunenko at oracle.com <mailto:dmitry.fazunenko at 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.fazunenko at oracle.com <mailto:dmitry.fazunenko at 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)----------
>>>>>>>>>

More information about the hotspot-dev mailing list