RFR (M) : JDK-8016029 : test runtime/6878713/Test6878713.sh failed - round 2

[David Holmes]

On 20/08/2013 12:45 PM, Gary Collins wrote:
> Correct David.  They need to use compile jdk as you stated.
>
> Not sure what the jdktoolfinder is

It is one of the utilities in the testlibrary to make it easier to do 
all this multi-process stuff from Java. Problem is that JDKToolFinder 
only returns tools from the test-jdk. But if the tool (javac, jar, jcmd, 
jps etc) is only incidental to what is being tested it can (usually but 
not always) come from the compile-jdk.

David

>
>
> Gary
>
> Sent from my iPad
>
> On Aug 19, 2013, at 7:16 PM, David Holmes <david.holmes at oracle.com> wrote:
>
>> Hi Misha,
>>
>> I have a couple of issues with this.
>>
>> 1. Missing @run tag
>>
>> 2. The shell script uses the COMPILEJDK to find the jar tool, but you are using JDKToolFinder which uses the TESTJDK - which means your test will be limited to running only on a full JDK (not a JRE). I thought the testlibrary supported using either the test-jdk or the compile-jdk but I don't see that now. :(
>>
>> 3. Not new with your change, but the unpacked files don't get cleaned up.
>>
>> Thanks,
>> David
>>
>> On 20/08/2013 3:50 AM, Mikhailo Seledtsov wrote:
>>> This is a round 2 review for this fix.
>>> Please review.
>>>
>>> Original review request (dated 16-July-2013) with the same subject was:
>>> "
>>> Please review this test fix. The change is a fix for the original bug,
>>> plus an update to the test condition to reflect new JVM behavior and a
>>> rewrite of an sh script test in Java.
>>> JBS: https://jbs.oracle.com/bugs/browse/JDK-8016029
>>> (Old) Webrev: http://cr.openjdk.java.net/~mseledtsov/6878713/webrev.00/
>>> <http://cr.openjdk.java.net/%7Emseledtsov/6878713/webrev.00/>
>>> "
>>>
>>>
>>> Change since last round of review:
>>>      JVM run-time technical leadership has made a decision to keep the
>>> testcase.jar until a long-term solution is in place for handling "Java
>>> assembler"-based and byte-code-based test cases.
>>>
>>> Here is the updated webrev and comments:
>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8016029/webrev.01/
>>> <http://cr.openjdk.java.net/%7Emseledtsov/8016029/webrev.01/>
>>>      Testing: JPRT run for this test
>>> (2013-08-19-154813.mseledtsov.bugFixing01-hotspot-rt)
>>>
>>> Here are original notes:
>>> - the original behavior of the VM in this test case has changed, and the
>>> expected error message has changed as well. There is an extensive amount
>>> of information about this in the original bug, posted by Dan. The change
>>> was introduced in change-set: 4876:f75faf51e8c4 by Harold. Checking for
>>> the old expected messages has been removed from the test. This change is
>>> for JDK8 and forward, and no back-ports planned.
>>>
>>> - the flag -XX:+IgnoreUnrecognizedVMOptions has been removed to avoid
>>> masking a potential problem. If VM drops support for any of XX flags
>>> used in this test, it is better to know about it right away rather than
>>> mask it and silently pass/ignore.
>>>
>>> - the flag -XX:+PrintCommandLineFlags has been removed; if this
>>> information is desired, IMO it should be triggered either by a verbose
>>> mode of JTREG tools, or by hand, not by individual test(s).
>>>
>>> Round 2 notes:
>>> - the shell script has been replaced by Java
>>>
>>> - test has been moved to a new location, moving away from using bug
>>> numbers to using functional categories for test sub-folders
>>>
>>>
>>> Misha