On Fri, 27 Nov 2020 18:50:19 GMT, Ivan Šipka <isi...@openjdk.org> wrote:
> @iignatev could you please review? Thank you.
>
> note to self:
> jtreg test/jdk/java/lang/ProcessBuilder/InheritIO/InheritIoTest.java
> test/jdk/java/lang/SecurityManager/modules/CustomSecurityManagerTest.java
> test/jdk/java/lang/Thread/uncaughtexceptions/UncaughtExceptionsTest.java
> test/jdk/java/lang/annotation/loaderLeak/LoaderLeakTest.java
it would be much easier to review if it was 4 separate RFEs/RFRs.
many new files miss a newline at the end.
test/jdk/java/lang/ProcessBuilder/InheritIO/InheritIoTest.java line 60:
> 58: outputAnalyzer.shouldHaveExitValue(0);
> 59: outputAnalyzer.stderrShouldMatch(EXPECTED_RESULT_STDERR);
> 60: outputAnalyzer.stdoutShouldMatch(EXPECTED_RESULT_STDOUT);
you can chain these methods.
test/jdk/java/lang/ProcessBuilder/InheritIO/InheritIoTest.java line 61:
> 59: outputAnalyzer.stderrShouldMatch(EXPECTED_RESULT_STDERR);
> 60: outputAnalyzer.stdoutShouldMatch(EXPECTED_RESULT_STDOUT);
> 61: assertEquals(outputAnalyzer.getOutput(),EXPECTED_RESULT_STDOUT +
> EXPECTED_RESULT_STDERR);
I'd rather check stdout and stderr independently, this will also make checks at
59 and 60 redundant.
test/jdk/java/lang/Thread/uncaughtexceptions/UncaughtExceptionsTest.java line
70:
> 68: outputAnalyzer.shouldHaveExitValue(exitValue);
> 69: outputAnalyzer.stderrShouldMatch(stdErrMatch);
> 70: outputAnalyzer.stdoutShouldMatch(stdOutMatch);
why do you use `ShouldMatch` and not `ShouldContain` here?
test/jdk/java/lang/annotation/loaderLeak/LoaderLeakTest.java line 53:
> 51: Files.createDirectories(REPOSITORY_PATH);
> 52: List<String> classes = List.of("A.class", "B.class", "C.class");
> 53: for (String fileName : classes) {
is this really needed for the test to operate correctly? or can we just use
_regular_ `TEST_CLASSES` as CP?
-------------
Changes requested by iignatyev (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1484
