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

Register a Config instance to bootstrap tests #42715

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Aug 22, 2024

The goal of this PR is to bootstrap a Config instance very early in the testing process so it can be used in testing setup code, like in JUnit Extensions and listeners.

There is a new module called quarkus-junit5-config, replacing quarkus-junit5-properties. Right now, it is limited, but it can be expanded to support many of the JUnit configurations using our mechanisms.

We register the Config in the test classloader with a JUnit LauncherSessionListener. A call to ConfigProvider.getConfig can either return the test config or the Quarkus config, depending on the classloader. It is now consistent, whereas before, it could depend on where it was called (because ConfigProvider.getConfig would also create the config if unavailable).

Our config is now more tightly integrated with JUnit, so we don't require JUnit properties file to configure the QuarkusTestProfileAwareClassOrderer. We add a global JUnit ClassOrderer that can delegate to any other ClassOrderer provided by the Quarkus configuration.

We also redid the logging configuration on test. Since we now have a reliable configuration, there is no need to juggle the config as we did in BasicLoggingEnabler, and there is a simple LoggingSetupExtension that sets up the log.

Since config is more consistent with the testing code, we could remove many of the workarounds we had. For example:

There are still more workarounds spread through the code that we can remove after.

Breaking Changes:

  • Because Quarkus sets its own ClassOrderer, if a user wanted to supply its own, it would require to exclude io.quarkus:quarkus-junit5-properties from the test dependencies and provide a junit-platform.properties with the property junit.jupiter.testclass.order.default and FQN of the ordered as value. With this PR, the user only has to provide the orderer's FQN in quarkus.test.class-orderer.

@radcortez radcortez marked this pull request as draft August 23, 2024 08:06
@radcortez radcortez force-pushed the fix-10233 branch 3 times, most recently from 3f68dd5 to 8505fd2 Compare November 14, 2024 23:45
@radcortez radcortez marked this pull request as ready for review November 14, 2024 23:45
@radcortez radcortez force-pushed the fix-10233 branch 3 times, most recently from 3bba3cb to 158d6a4 Compare November 18, 2024 10:45
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 18, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 18, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 18, 2024
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/documentation labels Nov 21, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 21, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 21, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 21, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 21, 2024
@quarkusio quarkusio deleted a comment from quarkus-bot bot Nov 21, 2024
Copy link

github-actions bot commented Nov 21, 2024

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Dec 2, 2024

Unless there are any objects today, I am going to merge this tomorrow as it really cleans up things

@holly-cummins
Copy link
Contributor

The LauncherInterceptor is invoked a few times during test launch, but the one I've been looking at is before discovery. The class orderer is invoked after discovery. I'm not sure if that would affect the success of what you're doing.

I had a better look, and I'm now using a LauncherSessionListener to implement the logic, which seems to execute after the first interceptor call. It works just as well. This should allow you to change any Classloader stuff before we set the configuration (where we need the Classloader we want to use).

Oh, even better! Thanks for thinking about that interaction.

This comment has been minimized.

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Dec 5, 2024

Can we rebase this to see if the CI issue persists?

@radcortez
Copy link
Member Author

Yes, done!

@geoand
Copy link
Contributor

geoand commented Dec 5, 2024

🙏

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes and removed area/documentation area/core area/logging area/vertx area/scheduler area/dependencies Pull requests that update a dependency file labels Dec 5, 2024
Copy link

quarkus-bot bot commented Dec 5, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit adedb08.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Dec 5, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit adedb08.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingTestCase.testContinuousTestingScenario2 - History

  • io.quarkus.builder.BuildException: Build failure: Build failed due to errors [error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor\#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7 at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:124) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732) at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856) - java.lang.RuntimeException
java.lang.RuntimeException: 
io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7
	at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:124)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

com.example.grpc.hibernate.VertxBlockingRawTest.shouldAdd - History

  • Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

⚙️ Native Tests - HTTP

📦 integration-tests/rest-client-reactive

io.quarkus.it.rest.client.BasicTestIT.shouldCreateClientSpans - History

  • expected: <1> but was: <2> - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
	at io.quarkus.it.rest.client.BasicTest.shouldCreateClientSpans(BasicTest.java:216)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:789)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@geoand geoand merged commit a47ad60 into quarkusio:main Dec 5, 2024
54 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Dec 5, 2024
@quarkus-bot quarkus-bot bot added kind/bugfix kind/enhancement New feature or request and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing documentation or behaviour for quarkus.test.profile.tags Reexamine how we manage config in tests
5 participants