Unit tests for #2560

core/src/main/java/org/testng/ITestClass.java

@@ -45,13 +45,17 @@ public interface ITestClass extends IClass {
    */
   ITestNGMethod[] getBeforeClassMethods();
 
+  ITestNGMethod[] getBeforeClassMethods(String instance);
+
   /**
    * Returns all the methods that should be invoked after all the tests have been run on this class.
    *
    * @return All the methods that should be invoked after all the tests have been run on this class.
    */
   ITestNGMethod[] getAfterClassMethods();
 
+  ITestNGMethod[] getAfterClassMethods(String instance);
+
   /**
    * Returns All the methods that should be invoked before the suite is run.
    *

core/src/main/java/org/testng/TestClass.java

@@ -8,17 +8,13 @@
 import org.testng.xml.XmlClass;
 import org.testng.xml.XmlTest;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.HashMap;
+import java.util.*;
 
 /**
  * This class represents a test class: - The test methods - The configuration methods (test and
  * method) - The class file
  */
-class TestClass extends NoOpTestClass implements ITestClass, ITestClassConfigInfo {
+class TestClass extends NoOpTestClass implements ITestClass {
 
   private IAnnotationFinder annotationFinder = null;
   // The Strategy used to locate test methods (TestNG, JUnit, etc...)
@@ -31,21 +27,8 @@ class TestClass extends NoOpTestClass implements ITestClass, ITestClassConfigInf
   private final ITestObjectFactory objectFactory;
   private final String m_errorMsgPrefix;
 
-  private final Map<String, List<ITestNGMethod>> beforeClassConfig = new HashMap<>();
-
-  @Override
-  public List<ITestNGMethod> getAllBeforeClassMethods() {
-    return beforeClassConfig.values().parallelStream().reduce((a, b) -> {
-      List<ITestNGMethod> methodList = new ArrayList<>(a);
-      methodList.addAll(b);
-      return methodList;
-    }).orElse(Lists.newArrayList());
-  }
-
-  @Override
-  public List<ITestNGMethod> getInstanceBeforeClassMethods(String instance) {
-    return beforeClassConfig.get(instance);
-  }
+  private final Map<String, List<ITestNGMethod>> beforeClassMethodsPerInstance = new HashMap<>();
+  private final Map<String, List<ITestNGMethod>> afterClassMethodsPerInstance = new HashMap<>();
 
   private static final Logger LOG = Logger.getLogger(TestClass.class);
 
@@ -135,6 +118,32 @@ public void addInstance(Object instance) {
     iClass.addInstance(instance);
   }
 
+  @Override
+  public ITestNGMethod[] getBeforeClassMethods() {
+    return beforeClassMethodsPerInstance.values().stream()
+            .flatMap(Collection::stream)
+            .toArray(ITestNGMethod[]::new);
+  }
+
+  @Override
+  public ITestNGMethod[] getBeforeClassMethods(String instance) {
+    return beforeClassMethodsPerInstance.getOrDefault(instance, Collections.emptyList()).stream()
+            .toArray(ITestNGMethod[]::new);
+  }
+
+  @Override
+  public ITestNGMethod[] getAfterClassMethods() {
+    return afterClassMethodsPerInstance.values().stream()
+            .flatMap(Collection::stream)
+            .toArray(ITestNGMethod[]::new);
+  }
+
+  @Override
+  public ITestNGMethod[] getAfterClassMethods(String instance) {
+    return afterClassMethodsPerInstance.getOrDefault(instance, Collections.emptyList()).stream()
+            .toArray(ITestNGMethod[]::new);
+  }
+
   private void initMethods() {
     ITestNGMethod[] methods = testMethodFinder.getTestMethods(m_testClass, xmlTest);
     m_testMethods = createTestMethods(methods);
@@ -175,8 +184,6 @@ private void initMethods() {
               annotationFinder,
               true,
               xmlTest, eachInstance);
-      Object instance = IParameterInfo.embeddedInstance(eachInstance);
-      beforeClassConfig.put(instance.toString(), Arrays.asList(m_beforeClassMethods));
       m_afterClassMethods =
           ConfigurationMethod.createClassConfigurationMethods(
               objectFactory,
@@ -212,6 +219,12 @@ private void initMethods() {
               annotationFinder,
               false,
               xmlTest, eachInstance);
+
+      Object instance = IParameterInfo.embeddedInstance(eachInstance);
+      beforeClassMethodsPerInstance.computeIfAbsent(instance.toString(), ignored -> new ArrayList<>())
+              .addAll(Arrays.asList(m_beforeClassMethods));
+      afterClassMethodsPerInstance.computeIfAbsent(instance.toString(), ignored -> new ArrayList<>())
+              .addAll(Arrays.asList(m_afterClassMethods));
     }
   }
 

core/src/main/java/org/testng/TestRunner.java

@@ -478,7 +478,7 @@ private void initMethods() {
     //
     for (ITestClass tc : m_classMap.values()) {
       fixMethodsWithClass(tc.getTestMethods(), tc, testMethods);
-      fixMethodsWithClass(((ITestClassConfigInfo) tc).getAllBeforeClassMethods().toArray(new ITestNGMethod[0]), tc, beforeClassMethods);
+      fixMethodsWithClass(tc.getBeforeClassMethods(), tc, beforeClassMethods);
       fixMethodsWithClass(tc.getBeforeTestMethods(), tc, null);
       fixMethodsWithClass(tc.getAfterTestMethods(), tc, null);
       fixMethodsWithClass(tc.getAfterClassMethods(), tc, afterClassMethods);

core/src/main/java/org/testng/internal/NoOpTestClass.java

@@ -67,6 +67,11 @@ public ITestNGMethod[] getAfterClassMethods() {
     return m_afterClassMethods;
   }
 
+  @Override
+  public ITestNGMethod[] getAfterClassMethods(String instance) {
+    return new ITestNGMethod[0]; // TODO
+  }
+
   /** @return Returns the afterTestMethods. */
   @Override
   public ITestNGMethod[] getAfterTestMethods() {
@@ -79,6 +84,11 @@ public ITestNGMethod[] getBeforeClassMethods() {
     return m_beforeClassMethods;
   }
 
+  @Override
+  public ITestNGMethod[] getBeforeClassMethods(String instance) {
+    return new ITestNGMethod[0]; // TODO
+  }
+
   /** @return Returns the beforeTestMethods. */
   @Override
   public ITestNGMethod[] getBeforeTestMethods() {

core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java

@@ -169,7 +169,7 @@ protected void invokeBeforeClassMethods(ITestClass testClass, IMethodInstance mi
       }
       ConfigMethodArguments attributes = new Builder()
           .forTestClass(testClass)
-          .usingConfigMethodsAs(((ITestClassConfigInfo) testClass).getInstanceBeforeClassMethods(instance.toString()))
+          .usingConfigMethodsAs(testClass.getBeforeClassMethods(instance.toString()))
           .forSuite(m_testContext.getSuite().getXmlSuite())
           .usingParameters(m_parameters)
           .usingInstance(instance)
@@ -199,9 +199,9 @@ protected void invokeAfterClassMethods(ITestClass testClass, IMethodInstance mi)
         m_classMethodMap.getInvokedAfterClassMethods();
     Set<Object> instances =
         invokedAfterClassMethods.computeIfAbsent(testClass, key -> Sets.newHashSet());
-    Object inst = mi.getInstance();
-    if (!instances.contains(inst)) {
-      invokeInstances.add(inst);
+    Object instance = mi.getInstance();
+    if (!instances.contains(instance)) {
+      invokeInstances.add(instance);
     }
 
     for (IClassListener listener : m_listeners) {
@@ -210,7 +210,7 @@ protected void invokeAfterClassMethods(ITestClass testClass, IMethodInstance mi)
     for (Object invokeInstance : invokeInstances) {
       ConfigMethodArguments attributes = new Builder()
           .forTestClass(testClass)
-          .usingConfigMethodsAs(testClass.getAfterClassMethods())
+          .usingConfigMethodsAs(testClass.getAfterClassMethods(instance.toString()))
           .forSuite(m_testContext.getSuite().getXmlSuite())
           .usingParameters(m_parameters)
           .usingInstance(invokeInstance)

core/src/main/java/org/testng/junit/JUnitTestClass.java

@@ -98,12 +98,22 @@ public ITestNGMethod[] getBeforeClassMethods() {
     return m_beforeClass.toArray(new ITestNGMethod[m_beforeClass.size()]);
   }
 
+  @Override
+  public ITestNGMethod[] getBeforeClassMethods(String instance) {
+    return new ITestNGMethod[0]; // TODO
+  }
+
   /** @see org.testng.ITestClass#getAfterClassMethods() */
   @Override
   public ITestNGMethod[] getAfterClassMethods() {
     return m_afterClass.toArray(new ITestNGMethod[m_afterClass.size()]);
   }
 
+  @Override
+  public ITestNGMethod[] getAfterClassMethods(String instance) {
+    return new ITestNGMethod[0]; // TODO
+  }
+
   // features not supported by JUnit
   private static final ITestNGMethod[] EMPTY_METHODARRAY = new ITestNGMethod[0];
 

core/src/test/java/org/testng/internal/MethodInstanceTest.java

@@ -195,11 +195,21 @@ public ITestNGMethod[] getBeforeClassMethods() {
       return new ITestNGMethod[0];
     }
 
+    @Override
+    public ITestNGMethod[] getBeforeClassMethods(String instance) {
+      return new ITestNGMethod[0];
+    }
+
     @Override
     public ITestNGMethod[] getAfterClassMethods() {
       return new ITestNGMethod[0];
     }
 
+    @Override
+    public ITestNGMethod[] getAfterClassMethods(String instance) {
+      return new ITestNGMethod[0];
+    }
+
     @Override
     public ITestNGMethod[] getBeforeSuiteMethods() {
       return new ITestNGMethod[0];

core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java

@@ -32,11 +32,21 @@ public ITestNGMethod[] getBeforeClassMethods() {
     return new ITestNGMethod[0];
   }
 
+  @Override
+  public ITestNGMethod[] getBeforeClassMethods(String instance) {
+    return new ITestNGMethod[0];
+  }
+
   @Override
   public ITestNGMethod[] getAfterClassMethods() {
     return new ITestNGMethod[0];
   }
 
+  @Override
+  public ITestNGMethod[] getAfterClassMethods(String instance) {
+    return new ITestNGMethod[0];
+  }
+
   @Override
   public ITestNGMethod[] getBeforeSuiteMethods() {
     return new ITestNGMethod[0];

core/src/test/java/test/factory/github2560/ConstructorTestSample.java

@@ -0,0 +1,43 @@
+package test.factory.github2560;
+
+import org.testng.annotations.*;
+
+public class ConstructorTestSample {
+
+    private final int hashCode;
+
+    @Factory(dataProvider = "constructorArguments")
+    public ConstructorTestSample(int hashCode) {
+        this.hashCode = hashCode;
+    }
+
+    @DataProvider
+    public static Object[][] constructorArguments() {
+        return new Object[][]{{0}, {1}, {2}};
+    }
+
+    @BeforeClass
+    public void beforeClass() {
+    }
+
+    @BeforeMethod
+    public void beforeMethod() {
+    }
+
+    @Test
+    public void test() {
+    }
+
+    @AfterMethod
+    public void afterMethod() {
+    }
+
+    @AfterClass
+    public void afterClass() {
+    }
+
+    @Override
+    public int hashCode() {
+        return hashCode;
+    }
+}

core/src/test/java/test/factory/github2560/FactoryTestSample.java

@@ -0,0 +1,15 @@
+package test.factory.github2560;
+
+import org.testng.annotations.Factory;
+
+public class FactoryTestSample {
+
+    @Factory
+    public static Object[] factory() {
+        return new Object[]{
+                new TestClassSample(0),
+                new TestClassSample(1),
+                new TestClassSample(2)
+        };
+    }
+}

core/src/test/java/test/factory/github2560/Github2560Test.java

@@ -0,0 +1,50 @@
+package test.factory.github2560;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.testng.Assert;
+import org.testng.TestNG;
+import org.testng.annotations.Test;
+import test.SimpleBaseTest;
+
+import java.util.List;
+import java.util.Map;
+
+public class Github2560Test extends SimpleBaseTest {
+
+    @Test
+    public void staticFactory() {
+        TestNG testng = create(FactoryTestSample.class);
+        testng.setDefaultSuiteName("Static @Factory tests");
+        InvokedMethodListener invokedMethodListener = new InvokedMethodListener();
+        testng.addListener(invokedMethodListener);
+
+        testng.run();
+
+        Map<Integer, List<String>> expected = ImmutableMap.of(
+                0, ImmutableList.of("beforeClass", "beforeMethod", "test", "afterMethod", "afterClass"),
+                1, ImmutableList.of("beforeClass", "beforeMethod", "test", "afterMethod", "afterClass"),
+                2, ImmutableList.of("beforeClass", "beforeMethod", "test", "afterMethod", "afterClass")
+        );
+        Assert.assertEquals(invokedMethodListener.capturedBeforeInvocations, expected, "beforeInvocation");
+        Assert.assertEquals(invokedMethodListener.capturedAfterInvocations, expected, "afterInvocation");
+    }
+
+    @Test
+    public void constructorFactory() {
+        TestNG testng = create(ConstructorTestSample.class);
+        testng.setDefaultSuiteName("Constructor @Factory tests");
+        InvokedMethodListener invokedMethodListener = new InvokedMethodListener();
+        testng.addListener(invokedMethodListener);
+
+        testng.run();
+
+        Map<Integer, List<String>> expected = ImmutableMap.of(
+                0, ImmutableList.of("beforeClass", "beforeMethod", "test", "afterMethod", "afterClass"),
+                1, ImmutableList.of("beforeClass", "beforeMethod", "test", "afterMethod", "afterClass"),
+                2, ImmutableList.of("beforeClass", "beforeMethod", "test", "afterMethod", "afterClass")
+        );
+        Assert.assertEquals(invokedMethodListener.capturedBeforeInvocations, expected, "beforeInvocation");
+        Assert.assertEquals(invokedMethodListener.capturedAfterInvocations, expected, "afterInvocation");
+    }
+}

core/src/test/java/test/factory/github2560/InvokedMethodListener.java

@@ -0,0 +1,28 @@
+package test.factory.github2560;
+
+import org.testng.*;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class InvokedMethodListener implements IInvokedMethodListener {
+
+    final Map<Integer, List<String>> capturedBeforeInvocations = new ConcurrentHashMap<>();
+    final Map<Integer, List<String>> capturedAfterInvocations = new ConcurrentHashMap<>();
+
+    @Override
+    public void beforeInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context) {
+        Assert.assertSame(method.getTestMethod().getInstance(), testResult.getInstance());
+        capturedBeforeInvocations.computeIfAbsent(testResult.getInstance().hashCode(), ignored -> new ArrayList<>())
+                .add(method.getTestMethod().getMethodName());
+    }
+
+    @Override
+    public void afterInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context) {
+        Assert.assertSame(method.getTestMethod().getInstance(), testResult.getInstance());
+        capturedAfterInvocations.computeIfAbsent(testResult.getInstance().hashCode(), ignored -> new ArrayList<>())
+                .add(method.getTestMethod().getMethodName());
+    }
+}