On Wed, 30 Apr 2025 13:12:37 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> 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 21 additional >> commits since the last revision: >> >> - Eirik's review about code comments >> - fix comment typo >> - merge latest from master branch >> - 8355975: introduce a test for 8355975 >> - merge latest from master branch >> - merge latest from master branch >> - merge latest from master branch >> - merge latest from master branch >> - merge latest from master branch >> - improve code comment for ZipFile.zipCoder >> - ... and 11 more: https://git.openjdk.org/jdk/compare/9d9004bf...9a29b960 > > 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 of testng) https://bugs.openjdk.org/browse/CODETOOLS-7902676. Given this, I can't think of a different way to handle this situation. Would it be OK to continue using `assumeTrue(...)` for now? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r2068769646
