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

TestResourceScope.MATCHING_RESOURCES ignores configuration annotations from QuarkusTestResourceConfigurableLifecycleManager #44129

Closed
yrodiere opened this issue Oct 28, 2024 · 8 comments · Fixed by #44324
Labels
area/testing kind/bug Something isn't working
Milestone

Comments

@yrodiere
Copy link
Member

yrodiere commented Oct 28, 2024

Describe the bug

From what I can see, only the FQCN of the test resource lifecycle manager and the configured scope are taken into account when grouping together tests with matching resources:

public static Set<TestResourceManager.TestResourceComparisonInfo> testResourceComparisonInfo(Class<?> testClass,
Path testClassLocation) {
Set<TestResourceClassEntry> uniqueEntries = getUniqueTestResourceClassEntries(testClass, testClassLocation, null);
if (uniqueEntries.isEmpty()) {
return Collections.emptySet();
}
Set<TestResourceManager.TestResourceComparisonInfo> result = new HashSet<>(uniqueEntries.size());
for (TestResourceClassEntry entry : uniqueEntries) {
result.add(new TestResourceComparisonInfo(entry.testResourceLifecycleManagerClass().getName(), entry.getScope()));
}
return result;
}

This seems incomplete, as test resource lifecycle managers can implement QuarkusTestResourceConfigurableLifecycleManager to be customized per-test, like this:

https://github.com/quarkusio/search.quarkus.io/blob/41e3b9f2de465ea2dc74efaa46ee44b310d5cc08/src/test/java/io/quarkus/search/app/testsupport/QuarkusIOSample.java#L510-L531

https://github.com/yrodiere/search.quarkus.io/blob/42ebd004b6fa5d92960b0615fb8ead6b1b6cb726/src/test/java/io/quarkus/search/app/SearchServiceTest.java#L42

Expected behavior

Tests are considered to have matching resources if they have the same resource lifecycle managers with the same arguments.

Actual behavior

Tests are considered to have matching resources if they have the same resource lifecycle managers, regardless of arguments.

How to Reproduce?

Check out quarkusio/search.quarkus.io#345 , revert the last commit, and see how resources are reused despite the

Or, probably simpler, add a unit test next to the existing ones for @WithTestResource within Quarkus itself.

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

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

No response

Additional information

Related conversation: quarkusio/search.quarkus.io#345

PR introducing the feature: #42852

@yrodiere yrodiere added the kind/bug Something isn't working label Oct 28, 2024
Copy link

quarkus-bot bot commented Oct 28, 2024

/cc @radcortez (config)

@geoand
Copy link
Contributor

geoand commented Oct 28, 2024

I will have to check this out as I don't really remember any of the details :)

@geoand
Copy link
Contributor

geoand commented Oct 30, 2024

Tests are considered to have matching resources if they have the same resource lifecycle managers, regardless of arguments.

When you say arguments, I assume you mean the config annotation, right?

@yrodiere
Copy link
Member Author

Tests are considered to have matching resources if they have the same resource lifecycle managers, regardless of arguments.

When you say arguments, I assume you mean the config annotation, right?

Right.

geoand added a commit to geoand/quarkus that referenced this issue Oct 31, 2024
@geoand
Copy link
Contributor

geoand commented Oct 31, 2024

@yrodiere I opened #44215. I am pretty sure it doesn't cover this exact case It should cover this case too

geoand added a commit to geoand/quarkus that referenced this issue Oct 31, 2024
geoand added a commit to geoand/quarkus that referenced this issue Oct 31, 2024
@yrodiere
Copy link
Member Author

yrodiere commented Nov 5, 2024

So, it's moved to #44279.

I think that PR handles @WithTestResource#initArgs, but not configuration annotations... It takes io.quarkus.test.common.TestResourceManager.TestResourceClassEntry#args into account for comparison, but not io.quarkus.test.common.TestResourceManager.TestResourceClassEntry#configAnnotation.

To be clear when I talk about "configuration annotations" I mean this:

if (testResource instanceof QuarkusTestResourceConfigurableLifecycleManager
&& entry.getConfigAnnotation() != null) {
((QuarkusTestResourceConfigurableLifecycleManager<Annotation>) testResource)
.init(entry.getConfigAnnotation());

public static class AnnotationBasedQuarkusTestResource
implements QuarkusTestResourceConfigurableLifecycleManager<WithAnnotationBasedTestResource> {
private String key;
@Override
public void init(WithAnnotationBasedTestResource annotation) {
this.key = annotation.key();
}

... which is separate from @WithTestResource#initArgs. It's an alternative to it, I think.

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

Ah right, I had handled that, but forgot about it...

I'll open a new PR

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

#44324

gsmet pushed a commit to gsmet/quarkus that referenced this issue Nov 5, 2024
geoand added a commit that referenced this issue Nov 6, 2024
Take config annotation when trying to match test resources
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Nov 6, 2024
bschuhmann pushed a commit to bschuhmann/quarkus that referenced this issue Nov 16, 2024
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
2 participants