Introduce @WithTestResource as an analogous to @QuarkusTestResource(restrictAnnotatedClass=true)

[edeandrea]

Description

This is spawned from this thread - https://twitter.com/cowtowncoder/status/1736913697426338262

In the next major version of Quarkus (where we can make breaking changes) we should default restrictAnnotatedClass on the QuarkusTestResource annotation to true.

Implementation ideas

No response

[quarkus-bot]

/cc @geoand (testing)

[maxandersen]

I admit that reading the thread the default surprised me too and the attribute name is quite long vs for example just global=true|false.

So I grok the wish/request.

4.0 is not even planned yet except we know it's not happening anytime soon.

My question/concern is how big a breakage this will be and how many users will have to add restrictToAnnotatedClass=false to make their tests pass.

I do assume we can handle it in quarkus update but it's still problematic if everyone is affected.

Are there better alternatives?

• Deprecate the current and Introduce a new annotation with better defaults? Advantages: we don't break anyone and we can have @Deprecation notice stating the solution and update all docs to use the new. Disadvantage: an extra annotation

• Do the default change but have package level annotation or even config property to switch default ? Advantages: does not require users to change all their test code to try out 4.0. Disadvantage: I think we generally prefer to not have annotation behaviors encoded in config)

[geoand]

My question/concern is how big a breakage this will be and how many users will have to add restrictToAnnotatedClass=false to make their tests pass.

It could easily result in most tests of a testsuite failing

I think we generally prefer to not have annotation behaviors encoded in config)

Indeed

[geoand]

Another alternative would be to use a new annotation and deprecate the existing one

[Sanne]

Another alternative would be to use a new annotation and deprecate the existing one

+1

I don't think it's a good idea to defer such changes to a "major release", unless there's no alternative:

• it leads to piling up changes which need to be performed in a very specific, possibly short time frame
• it would need specific processes: who's keeping track of us not missing all the issues which need to happen when the next major is planned
• even for a major release, would be best to avoid breaking changes or at least have a nice migration path
• it defers improvements indefinitely

[edeandrea]

I agree with what everyone has said here. Not that I'm advocating for keeping the existing annotation, but using something like OpenRewrite would be pretty simple during a quarkus upgrade to keep people's current functionality on the existing annotation (anyone with @QuarkusTestResource without specifically specifying restrictToAnnotatedClass would be re-written to setting restrictToAnnotatedClass = false while anyone with @QuarkusTestResource while specifying restrictToAnnotatedClass set to any value would be left alone).

Deprecating an existing annotation and replacing it with another with all the same properties but just different defaults seems clunky to me.

@geoand is right - it could break an entire test suite, but I think in those cases those people would be of the same realization - that things were working in a way that they didn't know/understand it was working that way. I would venture to guess that the majority of people who specify restrictToAnnotatedClass=true do so because they understand the current behavior and don't want it.

[tqvarnst]

An alternative could be to retain the old behavior with a feature flag in the configuration. For example:

quarkus.testing.default.test-resources.as.global=true #Just a suggestion, feel free to suggest another config key.

With this one set, the default behavior returns to making TestResources global. If not set we restrict to annotated class only.

[geoand]

@tqvarnst, I think that's in effect one of the proposals @maxandersen put forth, but we have generally shied away from allowing such kind of behavioral change via configuration.

[geoand]

The more I think of it, the more I am in favor of introducing a new annotation and deprecating the old one.
Any suggestions for a new name?

[edeandrea]

Naming things is hard :) I think QuarkusTestResource is actually quite a good name as it clearly identifies what it does.

How about something like @UseTestResource? More of a verb than a noun.

[geoand]

@UseTestResource or @WithTestResource sound like nice to names to me.

[edeandrea]

I like @WithTestResource

[geoand]

Let's also ask @gsmet who is good with naming things

[edeandrea]

@geoand any problems with me taking this up?

[geoand]

Yeah, I think adding the new annotation makes sense!

[geoand]

I updated the title of the issue to reflect the latest status