On Tue, 25 Apr 2023 22:08:09 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review changes to the IntlTest (test framework) class. >> >> These changes include >> - Logging the actual exception + stack trace. Previously, the test framework >> would throw `InvocationTargetException` but hide the actual underlying >> exception of the failing test unless the test class was ran with -nothrow. >> - Sorting the tests, ensuring for any given run, the tests are ran in the >> same order. (In [JDK-8305853](https://bugs.openjdk.org/browse/JDK-8305853) >> it was discovered that on Windows, the test framework would execute tests in >> a random order each time). >> - Improving output. > > Justin Lu has updated the pull request incrementally with two additional > commits since the last revision: > > - Revert changes that may affect extending test classes > - Add back toHexString (used by some tests) Looks good, with minor nits: test/jdk/java/text/testlib/IntlTest.java line 84: > 82: for (Method testMethod : testsToRun) { > 83: int oldCount = errorCount; > 84: writeTestName(testMethod.getName()); Could cache the method name for later uses test/jdk/java/text/testlib/IntlTest.java line 134: > 132: indentLevel--; > 133: writeTestResult(errorCount); > 134: Would this mean the final result is only written with `nothrow` option? test/jdk/java/text/testlib/IntlTest.java line 228: > 226: protected int getErrorCount() { > 227: return errorCount; > 228: } No need to move the method ------------- PR Review: https://git.openjdk.org/jdk/pull/13655#pullrequestreview-1400872160 PR Review Comment: https://git.openjdk.org/jdk/pull/13655#discussion_r1177142651 PR Review Comment: https://git.openjdk.org/jdk/pull/13655#discussion_r1177141497 PR Review Comment: https://git.openjdk.org/jdk/pull/13655#discussion_r1177142257
