RFR: 8347712: IllegalStateException on multithreaded ZipFile access with non-UTF8 charset [v7]

[Lance Andersen]

On Wed, 30 Apr 2025 14:20:37 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> test/jdk/java/util/zip/ZipFile/ZipFileCharsetTest.java line 68:
>> 
>>> 66:         // ISO-8859-15 is not a standard charset in Java. We skip this test
>>> 67:         // when it is unavailable
>>> 68:         assumeTrue(Charset.availableCharsets().containsKey(ISO_8859_15_NAME),
>> 
>> I would suggest throwing SkippedException otherwise junit throws org.opentest4j.TestAbortedException  If I understand correctly
>
> Hello Lance, I was under the impression that the jtreg framework would mark an aborted junit test as skipped. Now that you mentioned this, I checked locally and in its current form, jtreg reports this test as:
> 
> 
> STARTED    ZipFileCharsetTest::testCachedZipFileSource 'testCachedZipFileSource()'
> org.opentest4j.TestAbortedException: Assumption failed: skipping test since ISO-8859-15 charset isn't available
> ABORTED    ZipFileCharsetTest::testCachedZipFileSource 'testCachedZipFileSource()' [37ms]
> 
> [ JUnit Containers: found 4, started 4, succeeded 4, failed 0, aborted 0, skipped 0]
> [ JUnit Tests: found 1, started 1, succeeded 0, failed 0, aborted 1, skipped 0]
> 
> 
> 
> 
> So it's being classified by jtreg as aborted instead of skipped. 
> 
> I then took your suggestion to throw `jtreg.SkippedException`:
> 
> 
> +import jtreg.SkippedException;
> -import static org.junit.jupiter.api.Assumptions.assumeTrue;
>  
> + * @library /test/lib
>   * @run junit ZipFileCharsetTest
>   */
>  public class ZipFileCharsetTest {
>  
>          // ISO-8859-15 is not a standard charset in Java. We skip this test
>          // when it is unavailable
> -        assumeTrue(Charset.availableCharsets().containsKey(ISO_8859_15_NAME),
> -                "skipping test since " + ISO_8859_15_NAME + " charset isn't available");
> +        if (!Charset.availableCharsets().containsKey(ISO_8859_15_NAME)) {
> +            throw new SkippedException("skipping test since " + ISO_8859_15_NAME
> +                    + " charset isn't available");
> +        }
> 
> 
> 
> That however makes jtreg mark this test as failed instead of skipped:
> 
> 
> STARTED    ZipFileCharsetTest::testCachedZipFileSource 'testCachedZipFileSource()'
> jtreg.SkippedException: skipping test since ISO-8859-15 charset isn't available
>     at ZipFileCharsetTest.testCachedZipFileSource(ZipFileCharsetTest.java:70)
>     at java.base/java.lang.reflect.Method.invoke(Method.java:565)
>     at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
>     at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
> FAILED     ZipFileCharsetTest::testCachedZipFileSource 'testCachedZipFileSource()' [28ms]
> JavaTest Message: JUnit Platform Failure(s): 1
> 
> [ JUnit Containers: found 4, started 4, succeeded 4, failed 0, aborted 0, skipped 0]
> [ JUnit Tests: found 1, started 1, succeeded 0, failed 1, aborted 0, skipped 0]
> 
> 
> So it looks like we have an issue here with jtreg when it runs a junit test and the test throws `jtreg.SkippedException`. There appears to be an open issue for that (although in context o...

Hi Jai,

It is the lessor of 2 evils WRT the reporting isn't it.    It is unfortunate that this feature is not working.  I think throwing the SkippedException is the right way to go long term but I assume this shows as a reported failure in Mach5 and in the case of assumeTrue it does not result in a failure being reported?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r2069306532