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

QuarkusTestResource not working with QuarkusMainLauncher #45868

Closed
SKreileder opened this issue Jan 27, 2025 · 12 comments · Fixed by #45935
Closed

QuarkusTestResource not working with QuarkusMainLauncher #45868

SKreileder opened this issue Jan 27, 2025 · 12 comments · Fixed by #45935
Labels
area/testing kind/bug Something isn't working
Milestone

Comments

@SKreileder
Copy link

Describe the bug

I have a Quarkus Picocli application that among other things takes the URL to a JSON Schema catalog. That URL is then verified to exist.
For testing, I set up a QuarkusTestResource of a WireMockServer that returns some test files that would return true if verified.

For the tests itself, I used a mix of the "@Launch" annotation and the QuarkusMainLauncher. I used the second option if I needed the server so I could get the port currently in use. If I didn't need the server, I just used "@Launch".

The problem now arises if I try to execute the tests:
Let's assume I have three tests a, b and c, while a and c use the QuarkusMainLauncher and b uses "@Launch".
When all are executed a will fail because the TestResource hasn't been executed, the error is that the Property used for saving the URL couldn't be found. b and c will run without issue.

Image

If I know disable b bot a and c will fail with the same error:

Image

Expected behavior

I'd expect it to not matter if I use "@Launch" or QuarkusMainLauncher in combination with a QuarkusTestResource.

Actual behavior

As I see it, a "@Launch" test is required to run before a QuarkusTestResource can be used with a test that needs QuarkusMainLauncher.

How to Reproduce?

Reproducer: https://github.com/SKreileder/exampleTestResourceQuarkusMainLauncher

Steps to reproduce:

  1. build the project as a normal maven project with either the native profile active or not, it does not matter.
  2. Execute the test to see the different cases:
    • Execute the test class "Example1" to see my first mentioned example with a failing and b and c being successful
    • Execute the test class "Example2" to see my second mentioned example with a and c failing
    • Execute all tests in the package "integrationtests" to see only Example1.a failing.

Output of uname -a or ver

Linux stefan-21k90009ge 6.11.11-1-MANJARO #1 SMP PREEMPT_DYNAMIC Thu, 05 Dec 2024 16:26:44 +0000 x86_64 GNU/Linux

Output of java -version

java 21.0.5 2024-10-15 LTS Java(TM) SE Runtime Environment Oracle GraalVM 21.0.5+9.1 (build 21.0.5+9-LTS-jvmci-23.1-b48) Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 21.0.5+9.1 (build 21.0.5+9-LTS-jvmci-23.1-b48, mixed mode, sharing)

Quarkus version or git rev

3.17.8

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937) Maven home: /usr/share/java/maven Java version: 21.0.5, vendor: Oracle Corporation, runtime: /home/stefank/.sdkman/candidates/java/21.0.5-graal Default locale: de_DE, platform encoding: UTF-8 OS name: "linux", version: "6.11.11-1-manjaro", arch: "amd64", family: "unix"

Additional information

No response

@SKreileder SKreileder added the kind/bug Something isn't working label Jan 27, 2025
@geoand
Copy link
Contributor

geoand commented Jan 27, 2025

Thanks for reporting!

I tried your sample but it doesn't seem to compile. Is it missing lombok or something?

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Jan 27, 2025
@SKreileder
Copy link
Author

My mistake, I removed Lombok to keep the reproducer as simple as possible, but managed to forget to commit some changes.

It should compile now.

@geoand geoand removed the triage/needs-feedback We are waiting for feedback. label Jan 28, 2025
@geoand
Copy link
Contributor

geoand commented Jan 28, 2025

@radcortez it seems that is caused by the fact that the properties hang around even after the test is executed.

What is the proper way to clear them with the changes that have been made?

@radcortez
Copy link
Member

What is the proper way to clear them with the changes that have been made?

I'm unsure if the problem is with the clear of the properties. When I execute the Example1, the @QuarkusTestResource executes for both b and c but not a, setting the properties and clearing them after the test.

@geoand
Copy link
Contributor

geoand commented Jan 28, 2025

When a test that uses @Launch is run, it works (as expected) and that has the side-effect of setting schema.server.url (again expected).
However a subsequent test is run that tried to access schema.server.url which would normally fail, now works because the previous test set the config property.

@radcortez
Copy link
Member

But not because the properties were not cleared, it is because io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start is executed again for that test.

@geoand
Copy link
Contributor

geoand commented Jan 28, 2025

That's now what I'm seeing. io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start is called after schema.server.url is read.

@radcortez
Copy link
Member

In the case of Example1 this is what I see:

  • test a executes fails
  • test bexecutes
    • io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start sets the property
    • test passes
    • io.quarkus.test.common.TestResourceManager#close clears the property
  • test c executes
    • io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start sets the property
    • test passes
    • io.quarkus.test.common.TestResourceManager#close clears the property

@geoand
Copy link
Contributor

geoand commented Jan 28, 2025

test c executes

  • io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start sets the property
  • test passes
  • io.quarkus.test.common.TestResourceManager#close clears the property

This is the one I don't see. I see io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start get executed after SchemaServer.getSchemaCatalog() is called (which is expected)

@radcortez
Copy link
Member

Is it? Because that is the piece setting the properties, which makes the configuration visible to the test.

The same test code in a does not call io.quarkus.test.common.QuarkusTestResourceLifecycleManager#start hence the test fails.

@radcortez
Copy link
Member

This is related to #27996. While there is a clear of the config during close:

public void close() {
if (!started) {
return;
}
started = false;
for (TestResourceStartInfo entry : allTestResources) {
try {
entry.getTestResource().stop();
} catch (Exception e) {
throw new RuntimeException("Unable to stop Quarkus test resource " + entry.getTestResource(), e);
}
}
configProperties.clear();
}
other pieces of the code is setting configuration using System:
// propagate Quarkus properties set from the build tool
Properties existingSysProps = System.getProperties();
for (String name : existingSysProps.stringPropertyNames()) {
if (name.startsWith("quarkus.")) {
additionalProperties.put(name, existingSysProps.getProperty(name));
}
}
additionalProperties.putAll(testProfileAndProperties.properties);
//also make the dev services props accessible from the test
Map<String, String> resourceManagerProps = new HashMap<>(QuarkusMainIntegrationTestExtension.devServicesProps);
// Allow override of dev services props by integration test extensions
resourceManagerProps.putAll(testResourceManager.start());
for (Map.Entry<String, String> i : resourceManagerProps.entrySet()) {
old.put(i.getKey(), System.getProperty(i.getKey()));
if (i.getValue() == null) {
System.clearProperty(i.getKey());
} else {
System.setProperty(i.getKey(), i.getValue());
}
}
additionalProperties.putAll(resourceManagerProps);
testResourceManager.inject(context.getRequiredTestInstance());
SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
TestConfig testConfig = config.getConfigMapping(TestConfig.class);
ArtifactLauncher<?> launcher = null;
ServiceLoader<ArtifactLauncherProvider> loader = ServiceLoader.load(ArtifactLauncherProvider.class);
for (ArtifactLauncherProvider launcherProvider : loader) {
if (launcherProvider.supportsArtifactType(artifactType, testConfig.integrationTestProfile())) {
launcher = launcherProvider.create(
new DefaultArtifactLauncherCreateContext(quarkusArtifactProperties, context, requiredTestClass,
devServicesLaunchResult));
break;
}
}
if (launcher == null) {
throw new IllegalStateException(
"Artifact type + '" + artifactType + "' is not supported by @QuarkusMainIntegrationTest");
}
launcher.includeAsSysProps(additionalProperties);
activateLogging();
return launcher.runToCompletion(args);
} finally {
for (Map.Entry<String, String> i : old.entrySet()) {
old.put(i.getKey(), System.getProperty(i.getKey()));
if (i.getValue() == null) {
System.clearProperty(i.getKey());
} else {
System.setProperty(i.getKey(), i.getValue());
}
}
try {
if (testResourceManager != null) {
testResourceManager.close();
}
} catch (Exception e) {
System.err.println("Unable to shutdown resource: " + e.getMessage());
}
}
and the clear does not seem to be working as expected.

We really need to remove this way of passing configuration. Even if we clear the properly correctly, it is going to be unreliable if the tests execute in parallel.

@geoand
Copy link
Contributor

geoand commented Jan 28, 2025

We really need to remove this way of passing configuration. Even if we clear the properly correctly, it is going to be unreliable if the tests execute in parallel.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants