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

Smartly apply configs for test classes #22073

Closed
GregJohnStewart opened this issue Dec 9, 2021 · 11 comments
Closed

Smartly apply configs for test classes #22073

GregJohnStewart opened this issue Dec 9, 2021 · 11 comments
Labels
area/testing kind/enhancement New feature or request triage/wontfix This will not be worked on

Comments

@GregJohnStewart
Copy link
Contributor

Description

For some context, see #22025

Essentially, I am asking that the server running for @QuarkusTest be more smartly configured, in reference to Profiles, Lifecycle Managers, and the Lifecycle Manager arguments.

In my use case, I have two profiles, that need different arguments passed to the LifecycleManager to have different things spun up for different tests. Important note here is that with some arguments, the config values given will break other tests. These breaking configs are applied to all tests.

The current solution is to apply restrictToAnnotatedClass = true to all tests that have the manager configured to provide the arguments that break things. However, this is an imperfect solution as it requires:

  • all tests for the different profile to have applied
  • all tests for the different profile to spin up resources again for each test class

I would suggest that the code handles when the Quarkus server is started back up based on @TestProfile also considers @QuarkusTestResource, where if that resource annotation is different then a new service with the new lifecycle manager setup is started. I feel the caveat of "you should order your tests appropriately" is acceptable, and something I have another enhancement request for here at #22072 to make easier.

Implementation ideas

No response

@quarkus-bot
Copy link

quarkus-bot bot commented Dec 9, 2021

/cc @geoand

@geoand
Copy link
Contributor

geoand commented Dec 10, 2021

We could perhaps do this, but I am -1 because IMHO it would confuse users even more about how @QuarkusTestResource is supposed to be used.
Unfortunately, @QuarkusTestResource applies to all tests (I say unfortunately because it's confusing), but if apply the behavior change requested, this would become even more confusing.

@GregJohnStewart
Copy link
Contributor Author

I might move then to make @QuarkusTestResource apply only to the test classes that it is assigned, as that is how it seems to be presented in docs (at least, that is how I interpreted it). It is odd behavior to have the annotation on one class for all tests IMHO

@geoand
Copy link
Contributor

geoand commented Dec 13, 2021

It is. It was a mistake that we can't take back however

@GregJohnStewart
Copy link
Contributor Author

That's what major version number are for, right? :)

@geoand
Copy link
Contributor

geoand commented Dec 14, 2021

Well, it's too disruptive a change to ever be made IMHO.

@GregJohnStewart
Copy link
Contributor Author

I would argue that it's important to fix these things; I'm not the only one that will face similar issues and for the sake of developer joy it's important to address them, and have procedure in place to support the transition. Everything can't work the same way forever, things change!
Respectfully my own opinion, of course!

@stuartwdouglas
Copy link
Member

Can't you just use io.quarkus.test.junit.QuarkusTestProfile#testResources to assign resources to each profile?

With regards to ordering we now have QuarkusTestProfileAwareClassOrderer to sort them by profile, so that should not be an issue.

@famod
Copy link
Member

famod commented Feb 6, 2022

@GregJohnStewart with Stuart's answer, is there still missing some important feature for you?

@GregJohnStewart
Copy link
Contributor Author

GregJohnStewart commented Feb 7, 2022

@stuartwdouglas could you point to docs on how to do this? Looks promising but can't find anything on how to utilize. Nevermind, think I found it. I think this would be a good way to get it to work, and provides a bit of flexibility, what was discussed is still important.

Sorry for the lateness, this fell through the cracks over the holidays

@geoand
Copy link
Contributor

geoand commented Jan 27, 2023

Closing as won't fix as per discussion above

@geoand geoand closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2023
@geoand geoand added the triage/wontfix This will not be worked on label Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/enhancement New feature or request triage/wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants