Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12 - Re-enabled distribution tests that were disabled. #8722

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Oct 18, 2022

Signed-off-by: Simone Bordet [email protected]

@sbordet sbordet requested a review from joakime October 18, 2022 09:39
Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, just open an issue about that bad ee9 dependency from jakarta with jpms

public void testSimpleWebAppWithJSPOnModulePath(String env) throws Exception
{
// Testing with env=ee9 is not possible because jakarta.transaction:1.x
// does not have a proper module-info.java, so JPMS resolution will fail.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a new issue needs to be filed for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new issue to what project?

I don't think it can be fixed, as jakarta.transaction:1.x should remain Java 8 compatible (so no module-info.java) as per JEE 9.
It only has an Automatic-Module-Name but that is not enough to run with full JPMS.

I don't think the JakartaEE project will modify the jar, since they have already fixed it in jakarta.transaction:2.x (and in fact, our ee10 tests pass without problems).

@sbordet sbordet merged commit 02a7f1c into jetty-12.0.x Oct 18, 2022
@sbordet sbordet deleted the jetty-12.0.x-reenable-distribution-tests branch October 18, 2022 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants