Merge pull request quarkusio#42852 from geoand/@WithTestResource-fixed

Change default behavior of `@WithTestResource`

test-framework/common/src/main/java/io/quarkus/test/common/TestResourceScope.java

@@ -0,0 +1,25 @@
+package io.quarkus.test.common;
+
+/**
+ * Defines how Quarkus behaves with regard to the application of the resource to this test and the testsuite in general
+ */
+public enum TestResourceScope {
+
+    /*
+     * The declaration order must be from the narrowest scope to the widest
+     */
+
+    /**
+     * Means that Quarkus will run the test in complete isolation, i.e. it will restart every time it finds such a resource
+     */
+    RESTRICTED_TO_CLASS,
+    /**
+     * Means that Quarkus will not restart when running consecutive tests that use the same resource
+     */
+    MATCHING_RESOURCE,
+
+    /**
+     * Means the resource applies to all tests in the testsuite
+     */
+    GLOBAL
+}

test-framework/common/src/main/java/io/quarkus/test/common/WithTestResource.java

@@ -13,11 +13,20 @@
 /**
  * Used to define a test resource, which can affect various aspects of the application lifecycle.
  * <p>
+ * As of Quarkus 3.16, the default behavior of the annotation (meaning that {@code scope} has not been set)
+ * is that test classes annotated with the same {@code WithTestResource} will <strong>not</strong> force a restart
+ * of Quarkus.
+ * <p>
+ * The equivalent behavior to {@code QuarkusTestResource(restrictToAnnotatedClass = false)} is to use
+ * {@code WithTestResource(scope = TestResourceScope.GLOBAL)},
+ * while the equivalent behavior to {@code QuarkusTestResource(restrictToAnnotatedClass = true)} is to use
+ * {@code WithTestResource(scope = TestResourceScope.RESTRICTED_TO_CLASS)},
+ * <p>
  * WARNING: this annotation, introduced in 3.13, caused some issues so it was decided to undeprecate
  * {@link QuarkusTestResource} and rework the behavior of this annotation. For now, we recommend not using it
  * until we improve its behavior.
  * <p>
- * Note: When using the {@code restrictToAnnotatedClass=true} (which is the default), each test that is annotated
+ * Note: When using the {@code scope=TestResourceScope.RESTRICTED_TO_CLASS}, each test that is annotated
  * with {@code @WithTestResource} will result in the application being re-augmented and restarted (in a similar fashion
  * as happens in dev-mode when a change is detected) in order to incorporate the settings configured by the annotation.
  * If there are many instances of the annotation used throughout the testsuite, this could result in slow test execution.
@@ -28,14 +37,6 @@
  * started <b>before</b> <b>any</b> test is run.
  * <p>
  * Note that test resources are never restarted when running {@code @Nested} test classes.
- * <p>
- * The only difference with {@link QuarkusTestResource} is that the default value for
- * {@link #restrictToAnnotatedClass()} {@code == true}.
- * </p>
- * <p>
- * This means that any resources managed by {@link #value()} apply to an individual test class or test profile, unlike with
- * {@link QuarkusTestResource} where a resource applies to all test classes.
- * </p>
  */
 @Target(ElementType.TYPE)
 @Retention(RetentionPolicy.RUNTIME)
@@ -61,15 +62,11 @@
     boolean parallel() default false;
 
     /**
-     * Whether this annotation should only be enabled if it is placed on the currently running test class or test profile.
-     * Note that this defaults to true for meta-annotations since meta-annotations are only considered
-     * for the current test class or test profile.
-     * <p>
-     * Note: When this is set to {@code true} (which is the default), the annotation {@code @WithTestResource} will result
-     * in the application being re-augmented and restarted (in a similar fashion as happens in dev-mode when a change is
-     * detected) in order to incorporate the settings configured by the annotation.
+     * Defines how Quarkus behaves with regard to the application of the resource to this test and the test-suite in general.
+     * The default is {@link TestResourceScope#MATCHING_RESOURCE} which means that if two tests are annotated with the same
+     * {@link WithTestResource} annotation, no restart will take place between tests.
      */
-    boolean restrictToAnnotatedClass() default true;
+    TestResourceScope scope() default TestResourceScope.MATCHING_RESOURCE;
 
     @Target(ElementType.TYPE)
     @Retention(RetentionPolicy.RUNTIME)

test-framework/common/src/test/java/io/quarkus/test/common/TestResourceManagerInjectorTest.java

@@ -29,7 +29,7 @@ void testTestInjector(Class<?> clazz) {
         Assertions.assertEquals("dummy", foo.dummy.value);
     }
 
-    @WithTestResource(value = UsingTestInjectorLifecycleManager.class, restrictToAnnotatedClass = false)
+    @WithTestResource(value = UsingTestInjectorLifecycleManager.class, scope = TestResourceScope.GLOBAL)
     public static class UsingInjectorTest {
     }
 

test-framework/common/src/test/java/io/quarkus/test/common/TestResourceManagerReloadTest.java

@@ -0,0 +1,75 @@
+package io.quarkus.test.common;
+
+import static io.quarkus.test.common.TestResourceManager.testResourcesRequireReload;
+import static io.quarkus.test.common.TestResourceScope.*;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.junit.jupiter.api.Test;
+
+import io.quarkus.test.common.TestResourceManager.TestResourceComparisonInfo;
+
+public class TestResourceManagerReloadTest {
+
+    @Test
+    public void emptyResources() {
+        assertFalse(testResourcesRequireReload(Collections.emptySet(), Set.of()));
+    }
+
+    @Test
+    public void differentCount() {
+        assertTrue(testResourcesRequireReload(Collections.emptySet(),
+                Set.of(new TestResourceComparisonInfo("test", RESTRICTED_TO_CLASS))));
+
+        assertTrue(testResourcesRequireReload(Set.of(new TestResourceComparisonInfo("test", RESTRICTED_TO_CLASS)),
+                Collections.emptySet()));
+    }
+
+    @Test
+    public void sameSingleRestrictedToClassResource() {
+        assertTrue(testResourcesRequireReload(
+                Set.of(new TestResourceComparisonInfo("test", RESTRICTED_TO_CLASS)),
+                Set.of(new TestResourceComparisonInfo("test", RESTRICTED_TO_CLASS))));
+    }
+
+    @Test
+    public void sameSingleMatchingResource() {
+        assertFalse(testResourcesRequireReload(
+                Set.of(new TestResourceComparisonInfo("test", MATCHING_RESOURCE)),
+                Set.of(new TestResourceComparisonInfo("test", MATCHING_RESOURCE))));
+    }
+
+    @Test
+    public void differentSingleMatchingResource() {
+        assertTrue(testResourcesRequireReload(
+                Set.of(new TestResourceComparisonInfo("test", MATCHING_RESOURCE)),
+                Set.of(new TestResourceComparisonInfo("test2", MATCHING_RESOURCE))));
+    }
+
+    @Test
+    public void sameMultipleMatchingResource() {
+        assertFalse(testResourcesRequireReload(
+                Set.of(
+                        new TestResourceComparisonInfo("test", MATCHING_RESOURCE),
+                        new TestResourceComparisonInfo("test2", MATCHING_RESOURCE),
+                        new TestResourceComparisonInfo("test3", GLOBAL)),
+                Set.of(new TestResourceComparisonInfo("test3", GLOBAL),
+                        new TestResourceComparisonInfo("test2", MATCHING_RESOURCE),
+                        new TestResourceComparisonInfo("test", MATCHING_RESOURCE))));
+    }
+
+    @Test
+    public void differentMultipleMatchingResource() {
+        assertTrue(testResourcesRequireReload(
+                Set.of(
+                        new TestResourceComparisonInfo("test", MATCHING_RESOURCE),
+                        new TestResourceComparisonInfo("test2", MATCHING_RESOURCE),
+                        new TestResourceComparisonInfo("test3", GLOBAL)),
+                Set.of(new TestResourceComparisonInfo("test3", GLOBAL),
+                        new TestResourceComparisonInfo("test2", MATCHING_RESOURCE),
+                        new TestResourceComparisonInfo("TEST", MATCHING_RESOURCE))));
+    }
+}

test-framework/common/src/test/java/io/quarkus/test/common/TestResourceManagerTest.java

@@ -49,8 +49,8 @@ void testParallelResourcesRunInParallel(Class<?> clazz) {
         Assertions.assertEquals("value2", props.get("key2"));
     }
 
-    @WithTestResource(value = FirstLifecycleManager.class, restrictToAnnotatedClass = false)
-    @WithTestResource(value = SecondLifecycleManager.class, restrictToAnnotatedClass = false)
+    @WithTestResource(value = FirstLifecycleManager.class, scope = TestResourceScope.GLOBAL)
+    @WithTestResource(value = SecondLifecycleManager.class, scope = TestResourceScope.GLOBAL)
     public static class MyTest {
     }
 
@@ -99,8 +99,8 @@ public void stop() {
         }
     }
 
-    @WithTestResource(value = FirstSequentialQuarkusTestResource.class, restrictToAnnotatedClass = false)
-    @WithTestResource(value = SecondSequentialQuarkusTestResource.class, restrictToAnnotatedClass = false)
+    @WithTestResource(value = FirstSequentialQuarkusTestResource.class, scope = TestResourceScope.GLOBAL)
+    @WithTestResource(value = SecondSequentialQuarkusTestResource.class, scope = TestResourceScope.GLOBAL)
     public static class SequentialTestResourcesTest {
     }
 
@@ -150,8 +150,8 @@ public int order() {
         }
     }
 
-    @WithTestResource(value = FirstParallelQuarkusTestResource.class, parallel = true, restrictToAnnotatedClass = false)
-    @WithTestResource(value = SecondParallelQuarkusTestResource.class, parallel = true, restrictToAnnotatedClass = false)
+    @WithTestResource(value = FirstParallelQuarkusTestResource.class, parallel = true, scope = TestResourceScope.GLOBAL)
+    @WithTestResource(value = SecondParallelQuarkusTestResource.class, parallel = true, scope = TestResourceScope.GLOBAL)
     public static class ParallelTestResourcesTest {
     }
 
@@ -257,7 +257,7 @@ public void stop() {
         }
     }
 
-    @WithTestResource(value = AnnotationBasedQuarkusTestResource.class, restrictToAnnotatedClass = false)
+    @WithTestResource(value = AnnotationBasedQuarkusTestResource.class, scope = TestResourceScope.GLOBAL)
     @Retention(RetentionPolicy.RUNTIME)
     @Target(ElementType.TYPE)
     @Repeatable(WithAnnotationBasedTestResource.List.class)

test-framework/junit5/src/main/java/io/quarkus/test/junit/AbstractJvmQuarkusTestExtension.java

@@ -5,7 +5,6 @@
 import static io.quarkus.test.common.PathTestHelper.getTestClassesLocation;
 
 import java.io.IOException;
-import java.lang.annotation.Annotation;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -39,10 +38,8 @@
 import io.quarkus.paths.PathList;
 import io.quarkus.runtime.LaunchMode;
 import io.quarkus.test.common.PathTestHelper;
-import io.quarkus.test.common.QuarkusTestResource;
 import io.quarkus.test.common.RestorableSystemProperties;
 import io.quarkus.test.common.TestClassIndexer;
-import io.quarkus.test.common.WithTestResource;
 
 public class AbstractJvmQuarkusTestExtension extends AbstractQuarkusTestWithContextExtension {
 
@@ -270,43 +267,6 @@ private Class<? extends QuarkusTestProfile> findTestProfileAnnotation(Class<?> c
         return null;
     }
 
-    protected static boolean hasPerTestResources(ExtensionContext extensionContext) {
-        return hasPerTestResources(extensionContext.getRequiredTestClass());
-    }
-
-    public static boolean hasPerTestResources(Class<?> requiredTestClass) {
-        while (requiredTestClass != Object.class) {
-            for (WithTestResource testResource : requiredTestClass.getAnnotationsByType(WithTestResource.class)) {
-                if (testResource.restrictToAnnotatedClass()) {
-                    return true;
-                }
-            }
-
-            for (QuarkusTestResource testResource : requiredTestClass.getAnnotationsByType(QuarkusTestResource.class)) {
-                if (testResource.restrictToAnnotatedClass()) {
-                    return true;
-                }
-            }
-            // scan for meta-annotations
-            for (Annotation annotation : requiredTestClass.getAnnotations()) {
-                // skip TestResource annotations
-                var annotationType = annotation.annotationType();
-
-                if ((annotationType != WithTestResource.class) && (annotationType != QuarkusTestResource.class)) {
-                    // look for a TestResource on the annotation itself
-                    if ((annotationType.getAnnotationsByType(WithTestResource.class).length > 0)
-                            || (annotationType.getAnnotationsByType(QuarkusTestResource.class).length > 0)) {
-                        // meta-annotations are per-test scoped for now
-                        return true;
-                    }
-                }
-            }
-            // look up
-            requiredTestClass = requiredTestClass.getSuperclass();
-        }
-        return false;
-    }
-
     protected static class PrepareResult {
         protected final AugmentAction augmentAction;
         protected final QuarkusTestProfile profileInstance;

test-framework/junit5/src/main/java/io/quarkus/test/junit/IntegrationTestUtil.java

@@ -8,8 +8,6 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.UncheckedIOException;
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.net.URI;
@@ -19,10 +17,7 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.CodeSource;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
@@ -155,38 +150,6 @@ static TestProfileAndProperties determineTestProfileAndProperties(Class<? extend
         return new TestProfileAndProperties(testProfile, properties);
     }
 
-    /**
-     * Since {@link TestResourceManager} is loaded from the ClassLoader passed in as an argument,
-     * we need to convert the user input {@link QuarkusTestProfile.TestResourceEntry} into instances of
-     * {@link TestResourceManager.TestResourceClassEntry}
-     * that are loaded from that ClassLoader
-     */
-    static <T> List<T> getAdditionalTestResources(
-            QuarkusTestProfile profileInstance, ClassLoader classLoader) {
-        if ((profileInstance == null) || profileInstance.testResources().isEmpty()) {
-            return Collections.emptyList();
-        }
-
-        try {
-            Constructor<?> testResourceClassEntryConstructor = Class
-                    .forName(TestResourceManager.TestResourceClassEntry.class.getName(), true, classLoader)
-                    .getConstructor(Class.class, Map.class, Annotation.class, boolean.class);
-
-            List<QuarkusTestProfile.TestResourceEntry> testResources = profileInstance.testResources();
-            List<T> result = new ArrayList<>(testResources.size());
-            for (QuarkusTestProfile.TestResourceEntry testResource : testResources) {
-                T instance = (T) testResourceClassEntryConstructor.newInstance(
-                        Class.forName(testResource.getClazz().getName(), true, classLoader), testResource.getArgs(),
-                        null, testResource.isParallel());
-                result.add(instance);
-            }
-
-            return result;
-        } catch (Exception e) {
-            throw new IllegalStateException("Unable to handle profile " + profileInstance.getClass(), e);
-        }
-    }
-
     static void startLauncher(ArtifactLauncher launcher, Map<String, String> additionalProperties, Runnable sslSetter)
             throws IOException {
         launcher.includeAsSysProps(additionalProperties);

test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusIntegrationTestExtension.java

@@ -8,12 +8,13 @@
 import static io.quarkus.test.junit.IntegrationTestUtil.doProcessTestInstance;
 import static io.quarkus.test.junit.IntegrationTestUtil.ensureNoInjectAnnotationIsUsed;
 import static io.quarkus.test.junit.IntegrationTestUtil.findProfile;
-import static io.quarkus.test.junit.IntegrationTestUtil.getAdditionalTestResources;
 import static io.quarkus.test.junit.IntegrationTestUtil.getArtifactType;
 import static io.quarkus.test.junit.IntegrationTestUtil.getSysPropsToRestore;
 import static io.quarkus.test.junit.IntegrationTestUtil.handleDevServices;
 import static io.quarkus.test.junit.IntegrationTestUtil.readQuarkusArtifactProperties;
 import static io.quarkus.test.junit.IntegrationTestUtil.startLauncher;
+import static io.quarkus.test.junit.TestResourceUtil.testResourcesRequireReload;
+import static io.quarkus.test.junit.TestResourceUtil.TestResourceManagerReflections.copyEntriesFromProfile;
 
 import java.io.Closeable;
 import java.io.File;
@@ -75,7 +76,6 @@ public class QuarkusIntegrationTestExtension extends AbstractQuarkusTestWithCont
     private static Throwable firstException; //if this is set then it will be thrown from the very first test that is run, the rest are aborted
 
     private static Class<?> currentJUnitTestClass;
-    private static boolean hasPerTestResources;
 
     private static Map<String, String> devServicesProps;
     private static String containerNetworkId;
@@ -154,9 +154,9 @@ private QuarkusTestExtensionState ensureStarted(ExtensionContext extensionContex
             currentJUnitTestClass = extensionContext.getRequiredTestClass();
         }
         // we reload the test resources if we changed test class and if we had or will have per-test test resources
-        boolean reloadTestResources = isNewTestClass
-                && (hasPerTestResources || QuarkusTestExtension.hasPerTestResources(extensionContext));
-        if ((state == null && !failedBoot) || wrongProfile || reloadTestResources) {
+        boolean reloadTestResources = false;
+        if ((state == null && !failedBoot) || wrongProfile || (reloadTestResources = isNewTestClass
+                && TestResourceUtil.testResourcesRequireReload(state, extensionContext.getRequiredTestClass()))) {
             if (wrongProfile || reloadTestResources) {
                 if (state != null) {
                     try {
@@ -217,15 +217,15 @@ private QuarkusTestExtensionState doProcessStart(Properties quarkusArtifactPrope
             TestProfileAndProperties testProfileAndProperties = determineTestProfileAndProperties(profile, sysPropRestore);
 
             testResourceManager = new TestResourceManager(requiredTestClass, quarkusTestProfile,
-                    getAdditionalTestResources(testProfileAndProperties.testProfile,
+                    copyEntriesFromProfile(testProfileAndProperties.testProfile,
                             context.getRequiredTestClass().getClassLoader()),
                     testProfileAndProperties.testProfile != null
                             && testProfileAndProperties.testProfile.disableGlobalTestResources(),
                     devServicesProps, containerNetworkId == null ? Optional.empty() : Optional.of(containerNetworkId));
             testResourceManager.init(
                     testProfileAndProperties.testProfile != null ? testProfileAndProperties.testProfile.getClass().getName()
                             : null);
-            hasPerTestResources = testResourceManager.hasPerTestResources();
+
             if (isCallbacksEnabledForIntegrationTests()) {
                 populateCallbacks(requiredTestClass.getClassLoader());
             }