RFR 8067437: New tests for mJRE feature removal.

Andrey Nazarov andrey.x.nazarov at oracle.com
Mon Jan 12 09:25:21 UTC 2015


Hi Kumar,

I've updated patch with your comments.
http://cr.openjdk.java.net/~anazarov/8067437/webrev.02/

Also see comments inline.
Thank you for review.

On 23.12.2014 18:28, Kumar Srinivasan wrote:
> Missed clarifying this, I will be sponsoring this patch.
>
> Kumar
>
> On 12/23/2014 7:26 AM, Kumar Srinivasan wrote:
>> Neil,
>>
>> Since you contributed  the dev work, appreciate if you can provide some
>> feedback on the cleanup and new tests.
>>
>> Andrey,
>> Please see embedded comments below:
>>
>>> Please review new tests for the java launcher.
>>>
>>> New tests in MultipleJRE.java includes:
>>> 1. java -help must not contain information about obsolete flags
>>>
>>> "-version:", "-jre-restrict-search", "-jre-no-restrict-search"
>>>
>>> 2. java should emit error if obsolete flag is specified (combo test)
>>>
>>> 3. java should ignore manifest attributes: 
>>> "JRE-Version","JRE-Restrict-Search"
>>
>> MultipleJRE.java
>> Prefer to see braces for "for-while-loops"  lines 71, 72, its ok if 
>> you skip braces for
>> one-liner if-then-else clauses.
added braces
>>
>> I would also suggest dumping the TestResult separately upon failure 
>> to assist in debugging,
>> because it will be likely be "yours truly" who will triage future bugs.
>>
>>  112         if (!tr.testStatus) {
>> - 113             throw new RuntimeException("test case: failed\n" + 
>> tr.toString());
>> +                 System.out.println(tr);
>> +                 throw new RuntimeException("test case: failed\n" + 
>> cmd);
>>  114         }
>>
added System.out.println(tr) in several places.
>>
>>
>>>
>>> Removed tests:
>>>
>>> 1. Tests covered by MultipleJRE.java are removed in shell script 
>>> MultipleJRE.sh. Script can be ported to Java by another changeset.
>>
>> Amen, I want to see MultipleJRE.sh be whittled away and finally removed.
>>
>>>
>>> 2. Removed tests that check java argument parsing through re-exec 
>>> when another java version specified through flag -version:<id>
>>>
>>> Do we have tests for argument parsing in java launcher except 
>>> Arrrghs.java?
>>
>> Besides Arrrghs.java, there are tests in langtools
>> http://hg.openjdk.java.net/jdk9/dev/langtools/file/20475c78a0a6/test/tools/javac/Paths/wcMineField.sh 
>>
>> http://hg.openjdk.java.net/jdk9/dev/langtools/file/20475c78a0a6/test/tools/javac/Paths/MineField.sh
Arrrghs contains tests that check parsing of arguments with quotes and 
wildcard for Windows only. Do you why Unix systems are skipped?
>>
>>
>> So why is the BugId added to this test ? This test no longer tests 
>> any mJRE features ?
Removed the BugID. I planned to fix both issues by one changeset, but 
then decide to split changeset due to amount of work.
The Arrrghs.java no longer tests any mJRE features.
>>
>> Thanks for doing this!.
>>
>> Kumar
>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~anazarov/8067437/webrev.00/
>>>
>>>
>>>
>>>
>>> --Thanks, Andrey
>>
>
--Thanks, Andrey.




More information about the core-libs-dev mailing list