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

[David Holmes]

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