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

Test profile config properties can bleed into other test profiles #27996

Open
knutwannheden opened this issue Sep 16, 2022 · 6 comments · Fixed by #41957
Open

Test profile config properties can bleed into other test profiles #27996

knutwannheden opened this issue Sep 16, 2022 · 6 comments · Fixed by #41957
Assignees
Labels

Comments

@knutwannheden
Copy link
Contributor

Describe the bug

I have run into the problem (I've seen this in dev-mode, but don't know if it is specific to that), that config properties which are provided by test profiles (using QuarkusTestProfile#getConfigOverrides()) can bleed into later tests which don't use that profile.

I did some debugging and AFAICT the problem is in io.quarkus.test.junit.QuarkusTestExtension#doJavaStart(). In the called createAugmentor() method the RestorableSystemProperties.setProperties() method is called which sets the profile's config overrides as system properties and also returns a shutdown handler to restore the previous values. But doJavaStart() doesn't register these shutdown handlers until the very end and when executing the tests it is possible that these throw an exception (in my case I got an ExceptionInInitializerError which as its cause had an RuntimeException with the message Failed to start quarkus. As a consequence the shutdown handlers never get registered and the profile's config properties remain available as system properties.

Expected behavior

If profile config property overrides get set as system properties it must be ensured that these get properly reset / cleared, even if the test failed or throws exceptions.

Actual behavior

No response

How to Reproduce?

If requested I could look into provide a reproducer, but this may take some time.

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.12.2.Final

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

No response

Additional information

No response

@knutwannheden knutwannheden added the kind/bug Something isn't working label Sep 16, 2022
@knutwannheden
Copy link
Contributor Author

/cc @geoand

@geoand
Copy link
Contributor

geoand commented Sep 16, 2022

Seems like you have done some fine analysis. Would you be willing to provide a fix?

@knutwannheden
Copy link
Contributor Author

A simple fix will probably be easy. A test should also be doable, although I have only ever encountered this in continuous testing, which makes me wonder why I don't see it under normal Maven test execution.

I better fix may be to not rely on system properties, but a new config source. But for that I know too little about the inner workings of the Quarkus test framework.

@geoand
Copy link
Contributor

geoand commented Sep 19, 2022

Sure, that would be the test fix, but if we can just reset the system property in the proper place, we can take care of this.

knutwannheden added a commit to knutwannheden/quarkus that referenced this issue Sep 19, 2022
…reset

Instead of reverting the system properties which may get set by a Quarkus test as part of the `ExtensionState`'s `close()` operation, introduce a new `ShutdownTasks` resource which gets closed independently. Thus, it can perform tasks independently of whether the Quarkus application booted successfully or not.

Fixes quarkusio#27996
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 27, 2022

/cc @geoand

@radcortez
Copy link
Member

Reopen due to #42844

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants