8375080: The tools/jpackage/windows/Win8365790Test.java may fail with ClassNotFoundException: jtreg.SkippedException

[mmm-choi]

Win8365790Test calls TKit.throwSkippedException() when the test environment doesn't support Ctrl+C signal delivery.

In JDK 17/21, TKit.throwSkippedException() uses reflection (Class.forName("jtreg.SkippedException")) to load the exception class, which requires jtreg.SkippedException to be on the classpath.

public static RuntimeException throwSkippedException(String reason) {
    trace("Skip the test: " + reason);
    RuntimeException ex = ThrowingSupplier.toSupplier(
            () -> (RuntimeException) Class.forName("jtreg.SkippedException").getConstructor(
                    String.class).newInstance(reason)).get();
    currentTest.notifySkipped(ex);
    throw ex;
}

The test was missing the jtreg directives to build this class:

@library /test/lib
@build jtreg.SkippedException

This pattern is already used by other jpackage tests that call throwSkippedException() (e.g., SigningAppImageTest.java).

Note: JDK 25+ has this fixed differently via JDK-8352419 which hardcodes SkippedException directly in TKit.java.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• JDK-8375080 needs maintainer approval

Issue

• JDK-8375080: The tools/jpackage/windows/Win8365790Test.java may fail with ClassNotFoundException: jtreg.SkippedException (Bug - P4)

Reviewers

• Paul Hohensee (@phohensee - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/2606/head:pull/2606
$ git checkout pull/2606

Update a local copy of the PR:
$ git checkout pull/2606
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/2606/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2606

View PR using the GUI difftool:
$ git pr show -t 2606

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/2606.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mmm-choi! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[mlbridge]

Webrevs

• 00: Full (386abc26)

[jerboaa]

This seems to have been discussed in https://bugs.openjdk.org/browse/JDK-8376188 as well. cc @adamfarley

[alexeysemenyukoracle]

Isn't it the same issue as the one discussed in openjdk/jdk#29416?

The better fix would be to backport the part of the JDK-8352419 fix which hardcodes SkippedExeption class into test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java‎. This way, it will be consistent across all releases, and other jpackage tests will benefit from the properly working "skip the test" mechanism.

[adamfarley]

This issue should be fixed when #2605 gets merged.

It increases the minimum version of jtreg to 7.5.1+1, which doesn't have this issue.

[mmm-choi]

Just checked whether jtreg 7.5.1+1 fixes the issue and the test still fails with ClassNotFoundException for me.

I also ran a very simple test against 7.5.1 and it fails.

import jdk.jpackage.test.Annotations.Test;
import jdk.jpackage.test.TKit;

/*
 * @test
 * @summary Test TKit.throwSkippedException() without @library /test/lib
 * @library /test/jdk/tools/jpackage/helpers
 * @build jdk.jpackage.test.*
 * @build TKitSkipTest
 * @run main/othervm -Xmx512m jdk.jpackage.test.Main
 *  --jpt-run=TKitSkipTest
 */
public class TKitSkipTest {

    @Test
    public void test() {
        TKit.throwSkippedException("Testing TKit.throwSkippedException()");
    }
}

I verified that this simple test skips correctly in 25.

Please let me know how I can make this test pass with bumping Jtreg to 7.5.1+1.

[adamfarley]

Looking into this.

[alexeysemenyukoracle]

Please let me know how I can make this test pass with bumping Jtreg to 7.5.1+1.

I don't see how it will work unless Jtreg to 7.5.1+1 provides a copy of the jtreg.SkippedException class.

@sormuras can you comment on the matter?

[adamfarley]

I agree, Alexey. I believe you added a copy of the class to TKit.java in jdk25 here, to remove the dependency on SkippedException:

openjdk/jdk#24294

[sormuras]

jtreg doesn't ship with an own SkippedException - but it detects any other provided and thrown at test runtime. See for example: https://github.com/openjdk/jdk/blob/master/test/lib/jtreg/SkippedException.java

[alexeysemenyukoracle]

As @sormuras confirmed, jtreg doesn't provide a copy of jtreg.SkippedException exception.

This means, bumping Jtreg to 7.5.1+1 in JDK21 will not automatically fix the bug with the missing jtreg.SkippedException in the Win8365790Test test and in jpackage tests in general 😒

[alexeysemenyukoracle]

It increases the minimum version of jtreg to 7.5.1+1, which doesn't have this issue.

Any chance the tests were executed in a slightly different environment that looked good enough for Win8365790Test, that it didn't attempt to throw jtreg.SkippedException? This made it appear that a newer jtreg version had fixed the test failure.

[adamfarley]

It increases the minimum version of jtreg to 7.5.1+1, which doesn't have this issue.

Any chance the tests were executed in a slightly different environment

It's possible, yes. I thought upgrading to 7.5.1 would solve this issue, and it doesn't. Thank you for clarifying.

[mmm-choi]

Thank you everyone for reviewing!

I believe @alexeysemenyukoracle's solution is the correct approach. We should backport the relevant parts of openjdk/jdk#24294 that hardcode SkippedException, which will benefit other tests in JDK 21 and 17 that rely on this.

We have two options:

• Integrate this PR (since it's a straightforward fix) and then backport Alexey's fix separately
• Skip this PR and proceed directly to backporting Alexey's fix

I'd appreciate input on which approach to prefer. Both paths will achieve the same end result, so I'm happy to go with whatever makes most sense.

[alexeysemenyukoracle]

Skip this PR and proceed directly to backporting Alexey's fix

I support this variant. I don't see much point in an ad hoc fix for a specific test. We know the issue is not specific to Win8365790Test test; almost any jpackage test may fail when executed in an environment where it should be skipped. It just happens that the issue manifests in the Win8365790Test test. So let's proceed directly to a solution that fixes the problem in a generic way.

[openjdk]

⚠️ @mmm-choi This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.