-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add ability to control which classes are cloned #17306
Conversation
This workflow status is outdated as a new workflow run has been triggered. 🚫 This workflow run has been cancelled. Failing Jobs - Building c9c43dd
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spotted a few typos and asked a question.
@@ -152,6 +152,16 @@ | |||
@ConfigItem(defaultValue = "all") | |||
TestType type; | |||
|
|||
/** | |||
* If a class matches this pattern then it wil be cloned into the Quarkus ClassLoader even if it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* If a class matches this pattern then it wil be cloned into the Quarkus ClassLoader even if it | |
* If a class matches this pattern then it will be cloned into the Quarkus ClassLoader even if it |
@@ -152,6 +152,16 @@ | |||
@ConfigItem(defaultValue = "all") | |||
TestType type; | |||
|
|||
/** | |||
* If a class matches this pattern then it wil be cloned into the Quarkus ClassLoader even if it | |||
* is a parent first artifact. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* is a parent first artifact. | |
* is in a parent first artifact. |
@@ -950,8 +951,16 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation | |||
// because the test method runs from a class loaded from the TCCL | |||
List<Object> originalArguments = invocationContext.getArguments(); | |||
List<Object> argumentsFromTccl = new ArrayList<>(); | |||
String patternString = runningQuarkusApplication.getConfigValue("quarkus.test.class-clone-pattern", String.class) | |||
.orElse("java.*"); | |||
Pattern clonePattern = Pattern.compile(patternString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we should rather compile the pattern once globally rather than for each method?
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 8c9687b
Full information is available in the Build summary check run. Test Failures⚙️ JVM Tests - JDK 11 #📦 integration-tests/main✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ⚙️ JVM Tests - JDK 11 Windows #📦 integration-tests/main✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ⚙️ JVM Tests - JDK 16 #📦 integration-tests/main✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ |
e1431d1
to
d73605c
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building e1431d1
Full information is available in the Build summary check run. Test Failures⚙️ JVM Tests - JDK 11 #📦 integration-tests/main✖ ✖ ✖ ✖ ✖ ✖ ⚙️ JVM Tests - JDK 11 Windows #📦 integration-tests/main✖ ✖ ✖ ✖ ✖ ✖ ⚙️ JVM Tests - JDK 16 #📦 integration-tests/main✖ ✖ ✖ ✖ ✖ ✖ |
* This is important for collections which can contain objects from the Quarkus ClassLoader, but for | ||
* most parent first classes it will just cause problems. | ||
*/ | ||
@ConfigItem(defaultValue = "java.*") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartwdouglas sorry to raise this question so late but shouldn't it be java\..*
? Because with this, you also catch the Bean Validation classes starting with javax.
for instance and anything starting with java
at large.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch.
@@ -343,6 +346,9 @@ private ExtensionState doJavaStart(ExtensionContext context, Class<? extends Qua | |||
populateCallbacks(startupAction.getClassLoader()); | |||
|
|||
runningQuarkusApplication = startupAction.run(); | |||
String patternString = runningQuarkusApplication.getConfigValue("quarkus.test.class-clone-pattern", String.class) | |||
.orElse("java.*"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pattern should also be adjusted here if we change it.
This can interfere with other JUnit exists, such as Pact. Also make Pact parent first. Fixes quarkusio#9677
d73605c
to
9102276
Compare
This one is not backportable as is, it's just one big conflict. If really necessary, we need a specific PR. |
This can interfere with other JUnit exists, such
as Pact.
Fixes #9677