fix testng class fixtures for factory-created instances (fixes #896, via #1306)

baev · web-flow · commit 86e79b4962b8 · 2026-07-07T14:34:27.000+01:00
diff --git a/allure-testng/src/main/java/io/qameta/allure/testng/AllureTestNg.java b/allure-testng/src/main/java/io/qameta/allure/testng/AllureTestNg.java
@@ -76,8 +76,6 @@
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -164,9 +162,10 @@ public class AllureTestNg
             .withInitial(() -> UUID.randomUUID().toString());
 
     /**
-     * Store uuid for per-class scopes.
+     * Store uuid for per-class scopes: one scope per test class instance. Class fixtures run once per instance,
+     * so factory-created instances of the same class each get their own scope holding only that instance's tests.
      */
-    private final Map<ITestClass, String> classScopeUuidStorage = new ConcurrentHashMap<>();
+    private final Map<ClassInstanceKey, String> classScopeUuidStorage = new ConcurrentHashMap<>();
 
     /**
      * Store uuid for data provider scopes.
@@ -186,7 +185,6 @@ public class AllureTestNg
      * so they can no longer be reached through storage and are linked by these recorded uuids instead.
      */
     private final Map<GroupKey, List<String>> groupTestUuidStorage = new ConcurrentHashMap<>();
-    private final ReadWriteLock lock = new ReentrantReadWriteLock();
     private final AllureLifecycle lifecycle;
     private final AllureTestNgTestFilter testFilter;
 
@@ -264,11 +262,6 @@ public void onStart(final ITestContext context) {
         final String uuid = getUniqueUuid(context);
         getLifecycle().registerScope(scopeKey(uuid));
 
-        Stream.of(context.getAllTestMethods())
-                .map(ITestNGMethod::getTestClass)
-                .distinct()
-                .forEach(this::onBeforeClass);
-
         if (!config.isHideDisabledTests()) {
             context.getExcludedMethods().stream()
                     .filter(ITestNGMethod::isTest)
@@ -297,38 +290,34 @@ public void onFinish(final ITestContext context) {
         final String uuid = getUniqueUuid(context);
         getLifecycle().writeScope(scopeKey(uuid));
 
-        Stream.of(context.getAllTestMethods())
-                .map(ITestNGMethod::getTestClass)
-                .distinct()
-                .forEach(this::onAfterClass);
-
-        groupScopeUuidStorage.entrySet().removeIf(entry -> {
+        classScopeUuidStorage.entrySet().removeIf(entry -> {
             if (entry.getKey().context().equals(context)) {
                 getLifecycle().writeScope(scopeKey(entry.getValue()));
                 return true;
             }
             return false;
         });
-        groupTestUuidStorage.keySet().removeIf(key -> key.context().equals(context));
-    }
 
-    public void onBeforeClass(final ITestClass testClass) {
-        final String uuid = UUID.randomUUID().toString();
-        getLifecycle().registerScope(scopeKey(uuid));
-        setClassScope(testClass, uuid);
-    }
-
-    public void onAfterClass(final ITestClass testClass) {
-        getClassScope(testClass).ifPresent(uuid -> {
-            getLifecycle().writeScope(scopeKey(uuid));
-        });
+        final List<ITestClass> contextClasses = Stream.of(context.getAllTestMethods())
+                .map(ITestNGMethod::getTestClass)
+                .distinct()
+                .collect(Collectors.toList());
         dataProviderScopeUuidStorage.entrySet().removeIf(entry -> {
-            if (entry.getKey().getTestClass().equals(testClass)) {
+            if (contextClasses.stream().anyMatch(clazz -> entry.getKey().getTestClass().equals(clazz))) {
                 getLifecycle().writeScope(scopeKey(entry.getValue()));
                 return true;
             }
             return false;
         });
+
+        groupScopeUuidStorage.entrySet().removeIf(entry -> {
+            if (entry.getKey().context().equals(context)) {
+                getLifecycle().writeScope(scopeKey(entry.getValue()));
+                return true;
+            }
+            return false;
+        });
+        groupTestUuidStorage.keySet().removeIf(key -> key.context().equals(context));
     }
 
     @Override
@@ -345,10 +334,7 @@ public void onTestStart(final ITestResult testResult) {
 
         linkPendingBeforeMethodScopes(testKey(uuid));
 
-        Optional.of(testResult)
-                .map(ITestResult::getMethod)
-                .map(ITestNGMethod::getTestClass)
-                .ifPresent(clazz -> addTestToClassScope(clazz, uuid));
+        addTestToClassScope(testResult, uuid);
 
         Optional.of(testResult)
                 .map(ITestResult::getMethod)
@@ -357,6 +343,14 @@ public void onTestStart(final ITestResult testResult) {
         addTestToGroupScopes(testResult, uuid);
     }
 
+    private void addTestToClassScope(final ITestResult testResult, final String uuid) {
+        final ITestClass testClass = testResult.getMethod().getTestClass();
+        final String scopeUuid = getOrCreateClassScope(
+                testResult.getTestContext(), testClass, testResult.getInstance()
+        );
+        getLifecycle().addTestToScope(scopeKey(scopeUuid), testKey(uuid));
+    }
+
     private void addTestToGroupScopes(final ITestResult testResult, final String uuid) {
         final String[] groups = testResult.getMethod().getGroups();
         if (groups.length == 0) {
@@ -555,7 +549,7 @@ public void beforeInvocation(final IInvokedMethod method, final ITestResult test
             ifSuiteFixtureStarted(context.getSuite(), testMethod);
             ifTestFixtureStarted(context, testMethod);
             ifGroupsFixtureStarted(context, testMethod);
-            ifClassFixtureStarted(testMethod);
+            ifClassFixtureStarted(context, testMethod, testResult.getInstance());
             ifMethodFixtureStarted(testMethod);
         }
     }
@@ -569,17 +563,34 @@ private void ifSuiteFixtureStarted(final ISuite suite, final ITestNGMethod testM
         }
     }
 
-    private void ifClassFixtureStarted(final ITestNGMethod testMethod) {
+    private void ifClassFixtureStarted(final ITestContext context,
+                                       final ITestNGMethod testMethod,
+                                       final Object instance) {
         if (testMethod.isBeforeClassConfiguration()) {
-            getClassScope(testMethod.getTestClass())
-                    .ifPresent(parentUuid -> startBefore(parentUuid, testMethod));
+            startBefore(getOrCreateClassScope(context, testMethod.getTestClass(), instance), testMethod);
         }
         if (testMethod.isAfterClassConfiguration()) {
-            getClassScope(testMethod.getTestClass())
-                    .ifPresent(parentUuid -> startAfter(parentUuid, testMethod));
+            startAfter(getOrCreateClassScope(context, testMethod.getTestClass(), instance), testMethod);
         }
     }
 
+    /**
+     * Returns the uuid of the scope shared by the class fixtures and tests running on the given test class
+     * instance, registering it on first use. The scope is keyed by the real class rather than {@link ITestClass}:
+     * TestNG hands out different {@link ITestClass} references for invocation-count and data-provider clones of
+     * the same class, while the real class and the instance stay stable.
+     */
+    private String getOrCreateClassScope(final ITestContext context,
+                                         final ITestClass testClass,
+                                         final Object instance) {
+        final ClassInstanceKey key = new ClassInstanceKey(context, testClass.getRealClass(), instance);
+        return classScopeUuidStorage.computeIfAbsent(key, k -> {
+            final String uuid = UUID.randomUUID().toString();
+            getLifecycle().registerScope(scopeKey(uuid));
+            return uuid;
+        });
+    }
+
     private void ifTestFixtureStarted(final ITestContext context, final ITestNGMethod testMethod) {
         if (testMethod.isBeforeTestConfiguration()) {
             startBefore(getUniqueUuid(context), testMethod);
@@ -720,7 +731,10 @@ public void onConfigurationFailure(final ITestResult itr) {
 
         linkTestToScope(getUniqueUuid(itr.getTestContext()), uuid);
         linkTestToScope(getUniqueUuid(itr.getTestContext().getSuite()), uuid);
-        addTestToClassScope(itr.getMethod().getTestClass(), uuid);
+        linkTestToScope(
+                getOrCreateClassScope(itr.getTestContext(), itr.getMethod().getTestClass(), itr.getInstance()),
+                uuid
+        );
         if (itr.getMethod().isBeforeGroupsConfiguration()) {
             linkTestToScope(getOrCreateGroupsScope(itr.getTestContext(), itr.getMethod().getBeforeGroups()), uuid);
         }
@@ -1004,36 +1018,9 @@ private void addTestToDataProviderScope(final ITestNGMethod method, final String
         this.linkTestToScope(dataProviderScopeUuidStorage.get(method), childUuid);
     }
 
-    private void addTestToClassScope(final ITestClass clazz, final String childUuid) {
-        this.linkTestToScope(classScopeUuidStorage.get(clazz), childUuid);
-    }
-
     private void linkTestToScope(final String scopeUuid, final String childUuid) {
-        lock.writeLock().lock();
-        try {
-            if (nonNull(scopeUuid)) {
-                getLifecycle().addTestToScope(scopeKey(scopeUuid), testKey(childUuid));
-            }
-        } finally {
-            lock.writeLock().unlock();
-        }
-    }
-
-    private Optional<String> getClassScope(final ITestClass clazz) {
-        lock.readLock().lock();
-        try {
-            return Optional.ofNullable(classScopeUuidStorage.get(clazz));
-        } finally {
-            lock.readLock().unlock();
-        }
-    }
-
-    private void setClassScope(final ITestClass clazz, final String uuid) {
-        lock.writeLock().lock();
-        try {
-            classScopeUuidStorage.put(clazz, uuid);
-        } finally {
-            lock.writeLock().unlock();
+        if (nonNull(scopeUuid)) {
+            getLifecycle().addTestToScope(scopeKey(scopeUuid), testKey(childUuid));
         }
     }
 
@@ -1053,6 +1040,26 @@ private static boolean isClassAvailableOnClasspath(final String clazz) {
     private record CurrentTest(ITestResult source, String uuid) {
     }
 
+    /**
+     * The identity of a per-class scope: the test context and real class plus the specific instance its class
+     * fixtures and tests run on. The instance is compared by reference: test classes may override equals, and
+     * factory-created instances built from equal parameters must still get separate scopes.
+     */
+    private record ClassInstanceKey(ITestContext context, Class<?> realClass, Object instance) {
+        @Override
+        public boolean equals(final Object other) {
+            return other instanceof ClassInstanceKey key
+                    && Objects.equals(context, key.context)
+                    && Objects.equals(realClass, key.realClass)
+                    && instance == key.instance;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(context, realClass) * 31 + System.identityHashCode(instance);
+        }
+    }
+
     /**
      * The identity of a group fixture scope: the test context the group configuration methods run in, plus the set
      * of group names they declare.
diff --git a/allure-testng/src/test/java/io/qameta/allure/testng/AllureTestNgTest.java b/allure-testng/src/test/java/io/qameta/allure/testng/AllureTestNgTest.java
@@ -64,8 +64,10 @@
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -474,6 +476,62 @@ public void perClassFixtures(final XmlSuite.ParallelMode mode, final int threadC
                 .contains(test1.getUuid(), test2.getUuid());
     }
 
+    @AllureFeatures.Fixtures
+    @Issue("896")
+    @ParameterizedTest
+    @MethodSource("parallelConfiguration")
+    @DisplayName("Class fixtures of factory-created instances")
+    public void perInstanceClassFixtures(final XmlSuite.ParallelMode mode, final int threadCount) {
+        final AllureResults results = runTestNgSuites(
+                parallel(mode, threadCount),
+                "suites/factory-class-fixtures.xml"
+        );
+
+        final List<TestResult> testResults = results.getTestResults();
+        assertThat(testResults).hasSize(6);
+
+        // tests created from the same factory instance share the instance parameter value
+        final Map<String, String> instanceByUuid = testResults.stream()
+                .collect(
+                        Collectors.toMap(
+                                TestResult::getUuid,
+                                result -> result.getParameters().stream()
+                                        .filter(parameter -> "param".equals(parameter.getName()))
+                                        .map(Parameter::getValue)
+                                        .findFirst()
+                                        .orElse("")
+                        )
+                );
+
+        final List<TestResultContainer> classScopes = findContainersByFixtureName(
+                results.getTestResultContainers(), "beforeClass"
+        );
+        assertThat(classScopes)
+                .as("Each factory-created instance should get its own class scope")
+                .hasSize(3);
+
+        assertThat(classScopes).allSatisfy(scope -> {
+            assertThat(scope.getBefores())
+                    .extracting(FixtureResult::getName)
+                    .containsExactly("beforeClass");
+            assertThat(scope.getAfters())
+                    .extracting(FixtureResult::getName)
+                    .containsExactly("afterClass");
+            final Set<String> instances = scope.getChildren().stream()
+                    .map(instanceByUuid::get)
+                    .collect(Collectors.toSet());
+            assertThat(scope.getChildren()).hasSize(2);
+            assertThat(instances)
+                    .as("Class scope should hold the tests of a single instance")
+                    .hasSize(1);
+        });
+
+        assertThat(classScopes)
+                .flatExtracting(TestResultContainer::getChildren)
+                .as("Class scopes should partition all tests without overlap")
+                .containsExactlyInAnyOrderElementsOf(instanceByUuid.keySet());
+    }
+
     @AllureFeatures.Fixtures
     @Test
     @DisplayName("Before method fixture metadata propagates to the test")
diff --git a/allure-testng/src/test/java/io/qameta/allure/testng/samples/FactoryWithClassFixtures.java b/allure-testng/src/test/java/io/qameta/allure/testng/samples/FactoryWithClassFixtures.java
@@ -0,0 +1,66 @@
+/*
+ *  Copyright 2016-2026 Qameta Software Inc
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package io.qameta.allure.testng.samples;
+
+import io.qameta.allure.Step;
+import io.qameta.allure.testng.TestInstanceParameter;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Factory;
+import org.testng.annotations.Test;
+
+public class FactoryWithClassFixtures {
+
+    @TestInstanceParameter
+    private final String param;
+
+    @Factory(dataProvider = "parameters")
+    public FactoryWithClassFixtures(final String param) {
+        this.param = param;
+    }
+
+    @DataProvider
+    public static Object[][] parameters() {
+        return new Object[][]{{"first"}, {"second"}, {"third"}};
+    }
+
+    @BeforeClass
+    public void beforeClass() {
+        step();
+    }
+
+    @Test
+    public void test1() {
+        step();
+    }
+
+    @Test
+    public void test2() {
+        step();
+    }
+
+    @AfterClass
+    public void afterClass() {
+        step();
+    }
+
+    @Step
+    public void step() {
+
+    }
+
+}
diff --git a/allure-testng/src/test/resources/suites/factory-class-fixtures.xml b/allure-testng/src/test/resources/suites/factory-class-fixtures.xml
