Fix test iteration bug (#39915)

Fix test iteration bug

eng/versioning/external_dependencies.txt

@@ -255,7 +255,7 @@ org.junit.platform:junit-platform-testkit;1.9.3
 org.junit.vintage:junit-vintage-engine;5.9.3
 org.openjdk.jmh:jmh-core;1.37
 org.openjdk.jmh:jmh-generator-annprocess;1.37
-org.spockframework:spock-core;2.4-M1-groovy-4.0
+org.spockframework:spock-core;2.4-M4-groovy-4.0
 org.testng:testng;7.5.1
 org.awaitility:awaitility;4.2.0
 uk.org.lidalia:slf4j-test;1.2.0

eng/versioning/version_client.txt

@@ -469,6 +469,7 @@ com.azure.tools:azure-sdk-build-tool;1.0.0;1.1.0-beta.1
 # In the pom, the version update tag after the version should name the unreleased package and the dependency version:
 # <!-- {x-version-update;unreleased_com.azure:azure-core;dependency} -->
 
+unreleased_com.azure:azure-core-test;1.25.0-beta.1
 unreleased_com.azure:azure-core-amqp;2.10.0-beta.1
 
 # Released Beta dependencies: Copy the entry from above, prepend "beta_", remove the current

sdk/attestation/azure-security-attestation/src/test/java/com/azure/security/attestation/AttestationClientTestBase.java

@@ -27,10 +27,7 @@
 import org.bouncycastle.asn1.x509.BasicConstraints;
 import org.bouncycastle.asn1.x509.Extension;
 import org.bouncycastle.x509.X509V3CertificateGenerator;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.parallel.Isolated;
 import org.junit.jupiter.params.provider.Arguments;
 
@@ -85,6 +82,9 @@ public class AttestationClientTestBase extends TestProxyTestBase {
     protected void beforeTest() {
         super.beforeTest();
 
+        GlobalOpenTelemetry.resetForTest();
+        tracer = configureLoggingExporter(testContextManager.getTestName());
+
         if (interceptorManager.isPlaybackMode()) {
             interceptorManager.addMatchers(Collections.singletonList(new CustomMatcher()
                 .setHeadersKeyOnlyMatch(Collections.singletonList("Authorization"))));
@@ -122,21 +122,8 @@ public static void beforeAll() {
     }
 
     @Override
-    @BeforeEach
-    public void setupTest(TestInfo testInfo) {
-        GlobalOpenTelemetry.resetForTest();
-        super.setupTest(testInfo);
-        String testMethod = testInfo.getTestMethod().isPresent()
-            ? testInfo.getTestMethod().get().getName()
-            : testInfo.getDisplayName();
-        tracer = configureLoggingExporter(testMethod);
-    }
-
-    @Override
-    @AfterEach
-    public void teardownTest(TestInfo testInfo) {
+    public void afterTest() {
         GlobalOpenTelemetry.resetForTest();
-        super.teardownTest(testInfo);
     }
 
     @SuppressWarnings({"deprecation", "resource"})

sdk/core/azure-core-test/checkstyle-suppressions.xml

@@ -10,6 +10,9 @@
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.http.LocalTestServer.java" />
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.SyncAsyncExtension.java" />
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.TestBase.java" />
+  <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.junitextensions.LiveOnlyExtension.java" />
+  <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.junitextensions.PlaybackOnlyExtension.java" />
+  <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.junitextensions.TestContextManagerParameterResolver.java" />
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.TestProxyTestBase.java" />
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposed" files="com.azure.core.test.ThreadDumper.java" />
 

sdk/core/azure-core-test/spotbugs-exclude.xml

@@ -126,6 +126,10 @@
         <Class name="com.azure.core.test.models.TestProxyRecordingOptions$ProxyTransport" />
         <Method name="setCertificates" />
       </And>
+      <And>
+        <Class name="com.azure.core.test.TestBase" />
+        <Method name="setupTest" />
+      </And>
     </Or>
   </Match>
 

sdk/core/azure-core-test/src/main/java/com/azure/core/test/AzureTestWatcher.java

@@ -7,8 +7,8 @@
 import org.junit.jupiter.api.extension.BeforeTestExecutionCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
 
-import static com.azure.core.test.TestBase.getTestName;
 import static com.azure.core.test.TestBase.shouldLogExecutionStatus;
+import static com.azure.core.test.implementation.TestingHelpers.getTestName;
 
 /**
  * JUnit 5 extension class which reports on testing running and simple metrics about the test such as run time.

sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java

@@ -4,10 +4,9 @@
 
 import com.azure.core.http.HttpClient;
 import com.azure.core.http.HttpClientProvider;
-import com.azure.core.test.annotation.RecordWithoutRequestBody;
 import com.azure.core.test.http.PlaybackClient;
-import com.azure.core.test.implementation.TestIterationContext;
 import com.azure.core.test.implementation.TestingHelpers;
+import com.azure.core.test.junitextensions.TestContextManagerParameterResolver;
 import com.azure.core.test.utils.HttpURLConnectionHttpClient;
 import com.azure.core.test.utils.TestResourceNamer;
 import com.azure.core.util.Configuration;
@@ -20,30 +19,25 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.TestInfo;
-import org.junit.jupiter.api.extension.BeforeEachCallback;
-import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 import java.io.UncheckedIOException;
-import java.lang.reflect.Method;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
-import java.util.Objects;
-import java.util.Optional;
 import java.util.ServiceLoader;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Stream;
 
-import static com.azure.core.test.utils.TestUtils.toURI;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 /**
  * Base class for running live and playback tests using {@link InterceptorManager}.
  */
-public abstract class TestBase implements BeforeEachCallback {
+@ExtendWith(TestContextManagerParameterResolver.class)
+public abstract class TestBase {
     private static final String AZURE_TEST_DEBUG = "AZURE_TEST_DEBUG";
 
     // Environment variable name used to determine the TestMode.
@@ -115,11 +109,6 @@ public abstract class TestBase implements BeforeEachCallback {
      */
     protected TestContextManager testContextManager;
 
-    private ExtensionContext extensionContext;
-
-    @RegisterExtension
-    final TestIterationContext testIterationContext = new TestIterationContext();
-
     private long testStartTimeMillis;
 
     /**
@@ -137,41 +126,26 @@ public static void setupClass() {
         testMode = initializeTestMode();
     }
 
-    @Override
-    public void beforeEach(ExtensionContext extensionContext) {
-        this.extensionContext = extensionContext;
-    }
-
     /**
      * Sets-up the {@link TestBase#testResourceNamer} and {@link TestBase#interceptorManager} before each test case is
      * run. Then calls its implementing class to perform any other set-up commands.
      *
-     * @param testInfo {@link TestInfo} to retrieve test method name.
+     * @param testContextManager The {@link TestContextManager} managing information about this test.
      */
     @BeforeEach
-    public void setupTest(TestInfo testInfo) {
-        TestMode localTestMode = testMode;
-        // for unit tests of playback/recording in azure-core-test, allow for changing the mode per-test.
-        if (testInfo.getTags().contains("Record")) {
-            localTestMode = TestMode.RECORD;
-        } else if (testInfo.getTags().contains("Playback")) {
-            localTestMode = TestMode.PLAYBACK;
-        } else if (testInfo.getTags().contains("Live")) {
-            localTestMode = TestMode.LIVE;
-        }
+    public void setupTest(TestContextManager testContextManager) {
+        this.testContextManager = testContextManager;
+        // This check used to happen in the constructor for TestContextManager, but due to how JUnit performs parameter
+        // resolution for @BeforeEach methods, we need to check here. If it was left in the constructor, the test would
+        // fail instead of being skipped.
+        assumeTrue(testContextManager.didTestRun(), "Test does not allow playback and was ran in 'TestMode.PLAYBACK'");
 
-        String testName = getTestName(testInfo.getTestMethod(), testInfo.getDisplayName(), testInfo.getTestClass());
-        Path testClassPath = Paths.get(
-            toURI(testInfo.getTestClass().get().getResource(testInfo.getTestClass().get().getSimpleName() + ".class")));
-        this.testContextManager
-            = new TestContextManager(testInfo.getTestMethod().get(), localTestMode, isTestProxyEnabled(),
-                testInfo.getTestClass().get().getAnnotation(RecordWithoutRequestBody.class) != null, testClassPath);
-        testContextManager.setTestIteration(testIterationContext.getTestIteration());
-        ThreadDumper.addRunningTest(testName);
-        logger.info("Test Mode: {}, Name: {}", localTestMode, testName);
+        ThreadDumper.addRunningTest(testContextManager.getTrackerTestName());
+        logger.info("Test Mode: {}, Name: {}", testContextManager.getTestMode(),
+            testContextManager.getTrackerTestName());
 
         if (shouldLogExecutionStatus()) {
-            System.out.println("Starting test " + testName + ".");
+            System.out.println("Starting test " + testContextManager.getTrackerTestName() + ".");
             testStartTimeMillis = System.currentTimeMillis();
         }
 
@@ -187,11 +161,11 @@ public void setupTest(TestInfo testInfo) {
             // The supplier/consumer are used to retrieve/store variables over the wire.
             testResourceNamer = new TestResourceNamer(testContextManager, interceptorManager.getProxyVariableConsumer(),
                 interceptorManager.getProxyVariableSupplier());
-            if (localTestMode == TestMode.PLAYBACK && !testContextManager.doNotRecordTest()) {
+            if (testContextManager.getTestMode() == TestMode.PLAYBACK && !testContextManager.doNotRecordTest()) {
                 // We create the playback client here, so that it is available for returning recorded variables
                 // in a shared @BeforeEach in a test class.
                 interceptorManager.getPlaybackClient();
-            } else if (localTestMode == TestMode.RECORD && !testContextManager.doNotRecordTest()) {
+            } else if (testContextManager.getTestMode() == TestMode.RECORD && !testContextManager.doNotRecordTest()) {
                 // Similarly we create the record policy early so matchers/sanitizers can be added.
                 interceptorManager.getRecordPolicy();
             }
@@ -203,23 +177,21 @@ public void setupTest(TestInfo testInfo) {
 
     /**
      * Disposes of {@link InterceptorManager} and its inheriting class' resources.
-     *
-     * @param testInfo the injected testInfo
      */
     @AfterEach
-    public void teardownTest(TestInfo testInfo) {
-        String testName = getTestName(testInfo.getTestMethod(), testInfo.getDisplayName(), testInfo.getTestClass());
+    public void teardownTest() {
         if (shouldLogExecutionStatus()) {
             if (testStartTimeMillis > 0) {
                 long duration = System.currentTimeMillis() - testStartTimeMillis;
-                System.out.println("Finished test " + testName + " in " + duration + " ms.");
+                System.out
+                    .println("Finished test " + testContextManager.getTrackerTestName() + " in " + duration + " ms.");
             } else {
-                System.out.println("Finished test " + testName + ", duration unknown.");
+                System.out.println("Finished test " + testContextManager.getTrackerTestName() + ", duration unknown.");
             }
         }
 
         if (testContextManager != null) {
-            ThreadDumper.removeRunningTest(testName);
+            ThreadDumper.removeRunningTest(testContextManager.getTrackerTestName());
 
             if (testContextManager.didTestRun()) {
                 afterTest();
@@ -244,12 +216,12 @@ public TestMode getTestMode() {
      * @deprecated This method is deprecated as JUnit 5 provides a simpler mechanism to get the test method name through
      * {@link TestInfo}. Keeping this for backward compatability of other client libraries that still override this
      * method. This method can be deleted when all client libraries remove this method. See {@link
-     * #setupTest(TestInfo)}.
+     * #setupTest(TestContextManager)}.
      */
     @Deprecated
     protected String getTestName() {
-        if (extensionContext != null) {
-            return extensionContext.getTestMethod().map(Method::getName).orElse(null);
+        if (testContextManager != null) {
+            return testContextManager.getTestName();
         }
         return null;
     }
@@ -337,7 +309,7 @@ static TestMode initializeTestMode() {
      * Indicates whether the out of process test recording proxy is in use.
      * @return true if test proxy is to be used.
      */
-    protected static boolean isTestProxyEnabled() {
+    public static boolean isTestProxyEnabled() {
         return enableTestProxy;
     }
 
@@ -425,21 +397,6 @@ private static HttpClient getTestProxyHttpClient() {
             : httpClient);
     }
 
-    static String getTestName(Optional<Method> testMethod, String displayName, Optional<Class<?>> testClass) {
-        String testName = "";
-        String fullyQualifiedTestName = "";
-        if (testMethod.isPresent()) {
-            Method method = testMethod.get();
-            String className = testClass.map(Class::getName).orElse(method.getDeclaringClass().getName());
-            testName = method.getName();
-            fullyQualifiedTestName = className + "." + testName;
-        }
-
-        return Objects.equals(displayName, testName)
-            ? fullyQualifiedTestName
-            : fullyQualifiedTestName + "(" + displayName + ")";
-    }
-
     static boolean shouldLogExecutionStatus() {
         return Configuration.getGlobalConfiguration().get(AZURE_TEST_DEBUG, false);
     }

sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java

@@ -8,8 +8,6 @@
 import java.lang.reflect.Method;
 import java.nio.file.Path;
 
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
-
 /**
  * This class handles managing context about a test, such as custom testing annotations and verifying whether the test
  * is capable of running.
@@ -25,6 +23,7 @@ public class TestContextManager {
     private Integer testIteration;
     private final boolean skipRecordingRequestBody;
     private final Path testClassPath;
+    private final String trackerTestName;
 
     /**
      * Constructs a {@link TestContextManager} based on the test method.
@@ -48,6 +47,24 @@ public TestContextManager(Method testMethod, TestMode testMode) {
      */
     public TestContextManager(Method testMethod, TestMode testMode, boolean enableTestProxy,
         boolean recordWithoutRequestBodyClassAnnotation, Path testClassPath) {
+        this(testMethod, testMode, enableTestProxy, recordWithoutRequestBodyClassAnnotation, testClassPath,
+            testMethod.getName());
+    }
+
+    /**
+     * Constructs a {@link TestContextManager} based on the test method.
+     *
+     * @param testMethod Test method being ran.
+     * @param testMode The {@link TestMode} the test is running in.
+     * @param enableTestProxy True if the external test proxy is in use.
+     * @param recordWithoutRequestBodyClassAnnotation flag indicating if {@code RecordWithoutRequestBody} annotation
+     * present on test class.
+     * @param testClassPath the test class path
+     * @param trackerTestName The formatted test name used in logging and tracking its progress.
+     */
+    @SuppressWarnings("deprecation")
+    public TestContextManager(Method testMethod, TestMode testMode, boolean enableTestProxy,
+        boolean recordWithoutRequestBodyClassAnnotation, Path testClassPath, String trackerTestName) {
         this.testName = testMethod.getName();
         this.className = testMethod.getDeclaringClass().getSimpleName();
         this.testMode = testMode;
@@ -67,7 +84,7 @@ public TestContextManager(Method testMethod, TestMode testMode, boolean enableTe
         }
         this.testClassPath = testClassPath;
         this.testRan = !(skipInPlayback && testMode == TestMode.PLAYBACK);
-        assumeTrue(testRan, "Test does not allow playback and was ran in 'TestMode.PLAYBACK'");
+        this.trackerTestName = trackerTestName;
     }
 
     /**
@@ -154,7 +171,16 @@ public boolean didTestRun() {
      *
      * @param testIteration Test iteration.
      */
-    void setTestIteration(Integer testIteration) {
+    public void setTestIteration(Integer testIteration) {
         this.testIteration = testIteration;
     }
+
+    /**
+     * Gets the formatted name of the test used to log and track its progress.
+     *
+     * @return The formatted test name.
+     */
+    public String getTrackerTestName() {
+        return trackerTestName;
+    }
 }

sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestProxyTestBase.java

@@ -5,7 +5,6 @@
 
 import com.azure.core.test.utils.TestProxyManager;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.TestInfo;
 
 /**
  * Base class for running live and playback tests using test-proxy
@@ -25,10 +24,9 @@ public TestProxyTestBase() {
     /**
      * Before tests are executed, determines the test mode by reading the {@code AZURE_TEST_MODE} environment variable.
      * If it is not set, {@link TestMode#PLAYBACK}
-     * @param testInfo {@link TestInfo} to retrieve test related metadata.
      */
     @BeforeAll
-    public static void setupTestProxy(TestInfo testInfo) {
+    public static void setupTestProxy() {
         testMode = initializeTestMode();
         if (isTestProxyEnabled() && (testMode == TestMode.PLAYBACK || testMode == TestMode.RECORD)) {
             TestProxyManager.startProxy();

sdk/core/azure-core-test/src/main/java/com/azure/core/test/ThreadDumper.java

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 package com.azure.core.test;
 
+import com.azure.core.test.implementation.TestingHelpers;
 import com.azure.core.util.logging.ClientLogger;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeAllCallback;
@@ -153,6 +154,6 @@ static void removeRunningTest(String testName) {
     }
 
     private static String getFullTestName(ExtensionContext context) {
-        return TestBase.getTestName(context.getTestMethod(), context.getDisplayName(), context.getTestClass());
+        return TestingHelpers.getTestName(context.getTestMethod(), context.getDisplayName(), context.getTestClass());
     }
 }