On Thu, 18 Jun 2026 06:12:31 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this test-only change which proposes to improve 
>> the debuggability of the `test/jdk/sun/nio/cs/TestStringCodingUTF8.java` 
>> test?
>> 
>> This test fails intermittently in our CI in older update releases with 
>> errors like:
>> 
>> java.lang.RuntimeException: getBytes(csn) failed
>>     at TestStringCodingUTF8.test(TestStringCodingUTF8.java:111)
>>     at TestStringCodingUTF8.test(TestStringCodingUTF8.java:85)
>>     at TestStringCodingUTF8.main(TestStringCodingUTF8.java:41) 
>> 
>> 
>> It's not clear from the failures whether this is a test specific issue or 
>> some genuine issue in some specific release of the JDK. Since the test uses 
>> a `Random` instance to generate the data to test, it isn't easy to reproduce 
>> it either.
>> 
>> The change in this PR proposes to use the `RandomFactory` test library which 
>> is equipped with printing the seed used by the `Random` instance. If the 
>> test fails in future, the seed should help reproduce the test data that ran 
>> into the failure. The change also improves the error messages in the test 
>> and updates the `Collections.shuffle()` call to pass it the same `Random` 
>> instance that's being used in the test.
>> 
>> The test continues to pass with this change.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - print mismatched content
>  - merge latest from master branch
>  - 8386810: Improve debuggability of 
> test/jdk/sun/nio/cs/TestStringCodingUTF8.java

Thanks for the change, Jai. This should help debugging.

test/jdk/sun/nio/cs/TestStringCodingUTF8.java line 201:

> 199:             final StringBuilder sb = new StringBuilder();
> 200:             sb.append("Index=").append(i).append(", expected=");
> 201:             if (i > expected.length) {

Should this exclude `expected.length`? ie, `i >= expected.length`?

test/jdk/sun/nio/cs/TestStringCodingUTF8.java line 208:

> 206:             }
> 207:             sb.append(", actual=");
> 208:             if (i > actual.length) {

Same here.

-------------

PR Review: https://git.openjdk.org/jdk/pull/31548#pullrequestreview-4526600502
PR Review Comment: https://git.openjdk.org/jdk/pull/31548#discussion_r3437241450
PR Review Comment: https://git.openjdk.org/jdk/pull/31548#discussion_r3437242879

Reply via email to