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.
