Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources #13154

gregw · 2025-05-19T21:43:13Z

renamed jetty-ee to jetty-ee/jetty-ee-webapp
create jetty-ee/jetty-ee-test-resources

renamed jetty-ee to jetty-ee/jetty-ee-webapp create jetty-ee/jetty-ee-test-resources

joakime · 2025-05-19T22:34:11Z

Going to move the recent common annotations stuff to this new tree too?

gregw · 2025-05-19T23:38:20Z

Going to move the recent common annotations stuff to this new tree too?

The jetty-annotations module is not really ee related. It is a stand alone util and could be used with anything that parses annotations.

…1.x/testEeResources

renamed jetty-ee to jetty-ee/jetty-ee-webapp create jetty-ee/jetty-ee-test-resources

# Conflicts: # jetty-ee10/jetty-ee10-servlet/pom.xml # jetty-ee11/jetty-ee11-servlet/pom.xml

…1.x/testEeResources

gregw · 2025-05-26T08:14:24Z

@janbartel @olamy oh please please please review this quickly. It has taken me 2 weeks to get a clean build, so please review before it breaks again :)

joakime · 2025-05-27T18:39:29Z

jetty-core/jetty-ee/pom.xml

@@ -1,60 +1,20 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">


Interesting change.
Wonder if we should update the rest of the poms to match this new XSD location?

@olamy what's the recommended location now?

yup that's correct but better using https https://maven.apache.org/xsd/maven-4.0.0.xsd

I'll leave this as is (intellij did it) and we can change them all in another PR and also see if we can make IDEA do https

joakime

Lots of good changes here.
My comments are mostly things that can be done in a followup PR (if they sound like a good idea).

...tty-ee-test-resources/src/main/java/org/eclipse/jetty/ee/test/resources/TestEeResources.java

...ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/DefaultServletTest.java

...y-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ServletContextResourcesTest.java

joakime · 2025-05-27T18:50:44Z

...c/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/JakartaClientClassLoaderTest.java

@@ -183,7 +183,7 @@ public void websocketProvidedByServer() throws Exception
        assertThat(response.getStatus(), is(HttpStatus.OK_200));

        // The ContextClassLoader in the WebSocketClients onOpen was the WebAppClassloader.
-        assertThat(response.getContentAsString(), containsString("ContextClassLoader: oeje.WebAppClassLoader"));
+        assertThat(response.getContentAsString(), containsString("ContextClassLoader: oejew.WebAppClassLoader"));


Couldn't these kinds of assertions be changed to use TypeUtil to avoid this kind of change?

Suggested change

assertThat(response.getContentAsString(), containsString("ContextClassLoader: oejew.WebAppClassLoader"));

assertThat(response.getContentAsString(), containsString("ContextClassLoader: " + TypeUtil.toShortName(WebAppClassLoader.class));

jetty-ee9/jetty-ee9-annotations/src/main/java/module-info.java

...tty-ee-test-resources/src/main/java/org/eclipse/jetty/ee/test/resources/TestEeResources.java

olamy · 2025-05-28T10:22:54Z

jetty-core/jetty-ee/jetty-ee-webapp/pom.xml

+          <argLine>@{argLine} ${jetty.surefire.argLine} --add-reads org.eclipse.jetty.ee.webapp=org.eclipse.jetty.logging</argLine>
+          <excludes>
+            <!-- This class modifies internal JVM statics, and causes problems in parallel testing -->
+            <exclude>org.eclipse.jetty.ee.webapp.WebAppClassLoaderUrlStreamTest</exclude>


didn;t work using @Isolated?

This is the same thing that exists in 12.0.x, in the same technique.

https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-ee10/jetty-ee10-webapp/pom.xml#L99-L101

Can this be improved? Sure, does it need to be fixed here, now, and in this PR? no.

olamy · 2025-05-28T10:25:25Z

build/scripts/applyEE11ToEE10.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+git diff origin/jetty-12.1.x -- jetty-ee11 | sed -e 's/ee11/ee10/g' -e 's/EE11/EE10/g' | git apply


why do we need this script in git?

The same reason the opposite direction scripts (ee10 to ee11) exist in git.
This isn't a new concept.

Whenever merging 12.0.x forward to 12.1.x, the other script has been used to move changes from ee10 to ee11. Now that we are increasingly doing changes directly to ee11, it is good to have the ability to merge the other direction

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources

25769f1

renamed jetty-ee to jetty-ee/jetty-ee-webapp create jetty-ee/jetty-ee-test-resources

gregw requested review from janbartel and olamy May 19, 2025 21:43

gregw added this to Jetty 12.1.0 May 19, 2025

gregw added 2 commits May 20, 2025 07:50

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources

7c9f2e0

renamed jetty-ee to jetty-ee/jetty-ee-webapp create jetty-ee/jetty-ee-test-resources

Merge branch 'jetty-12.1.x' into fix/jetty-12.1.x/testEeResources

8057f45

gregw added the build-no-cache label May 19, 2025

gregw added 13 commits May 20, 2025 10:27

Merge remote-tracking branch 'origin/jetty-12.1.x' into fix/jetty-12.…

f8b4537

…1.x/testEeResources

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources

8aceb5b

renamed jetty-ee to jetty-ee/jetty-ee-webapp create jetty-ee/jetty-ee-test-resources

yet another WTF JPMS

d557a8b

Merge branch 'jetty-12.1.x' into fix/jetty-12.1.x/testEeResources

5a3e4c0

# Conflicts: # jetty-ee10/jetty-ee10-servlet/pom.xml # jetty-ee11/jetty-ee11-servlet/pom.xml

Fix test resource

4af0964

Fix test resource

dd68bda

Support Path from jar:file: better

2e0fdba

Fix module name

5deb23f

Fix module name

22d0caf

Merge remote-tracking branch 'origin/jetty-12.1.x' into fix/jetty-12.…

6081bb7

…1.x/testEeResources

Fix OSGi name

db962e9

Fix classloader dump test

692f7d3

Merge branch 'jetty-12.1.x' into fix/jetty-12.1.x/testEeResources

42057c7

joakime reviewed May 27, 2025

View reviewed changes

joakime approved these changes May 27, 2025

View reviewed changes

janbartel approved these changes May 28, 2025

View reviewed changes

...tty-ee-test-resources/src/main/java/org/eclipse/jetty/ee/test/resources/TestEeResources.java Outdated Show resolved Hide resolved

olamy reviewed May 28, 2025

View reviewed changes

olamy approved these changes May 28, 2025

View reviewed changes

updates from review

2f97aaf

joakime approved these changes May 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources #13154

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources #13154

gregw commented May 19, 2025

Uh oh!

joakime commented May 19, 2025

Uh oh!

gregw commented May 19, 2025

Uh oh!

gregw commented May 26, 2025

Uh oh!

joakime May 27, 2025

Uh oh!

olamy May 27, 2025 •

edited

Loading

Uh oh!

gregw May 29, 2025

Uh oh!

joakime left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joakime May 27, 2025

Uh oh!

Uh oh!

Uh oh!

olamy May 28, 2025

Uh oh!

joakime May 28, 2025

Uh oh!

olamy May 28, 2025

Uh oh!

joakime May 28, 2025

Uh oh!

gregw May 29, 2025

Uh oh!

Uh oh!

	assertThat(response.getContentAsString(), containsString("ContextClassLoader: oejew.WebAppClassLoader"));
	assertThat(response.getContentAsString(), containsString("ContextClassLoader: " + TypeUtil.toShortName(WebAppClassLoader.class));

		@@ -0,0 +1,3 @@
		#!/bin/sh

		git diff origin/jetty-12.1.x -- jetty-ee11 \| sed -e 's/ee11/ee10/g' -e 's/EE11/EE10/g' \| git apply

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources #13154

Are you sure you want to change the base?

Move EE test resources to jetty-core/jetty-ee/jetty-ee-test-resources #13154

Conversation

gregw commented May 19, 2025

Uh oh!

joakime commented May 19, 2025

Uh oh!

gregw commented May 19, 2025

Uh oh!

gregw commented May 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olamy May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakime left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

olamy May 27, 2025 •

edited

Loading