FirstTimeOnlyConfig methods + Listener invocations

Closes #2961

Author: krmahadevan

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-2961: "Unexpected value: 16" error when multiple beforeMethod config methods with firstTimeOnly property run before a test (Krishnan Mahadevan)
 Fixed: GITHUB-2904: Add location of docs Github to readme and contributions page (Mohsin Sackeer)
 Fixed: GITHUB-2934: Parallel Dataproviders & retries causes test result count to be skewed (Krishnan Mahadevan)
 Fixed: GITHUB-2925: Issue in ITestcontext.getAllTestMethods() with annotation @BeforeSuite (Krishnan Mahadevan)

testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java

@@ -317,15 +317,18 @@ public void invokeConfigurations(ConfigMethodArguments arguments) {
         runConfigurationListeners(testResult, arguments.getTestMethod(), true /* before */);
 
         Object newInstance = computeInstance(arguments.getInstance(), inst, tm);
-        if (isConfigMethodEligibleForScrutiny(tm)) {
+        boolean isFirstTimeOnlyConfigMethod = isConfigMethodEligibleForScrutiny(tm);
+        if (isFirstTimeOnlyConfigMethod) {
           if (m_executedConfigMethods.add(arguments.getTestMethod())) {
             invokeConfigurationMethod(newInstance, tm, parameters, testResult);
           }
         } else {
           invokeConfigurationMethod(newInstance, tm, parameters, testResult);
         }
         copyAttributesFromNativelyInjectedTestResult(parameters, arguments.getTestMethodResult());
-        runConfigurationListeners(testResult, arguments.getTestMethod(), false /* after */);
+        if (!isFirstTimeOnlyConfigMethod) {
+          runConfigurationListeners(testResult, arguments.getTestMethod(), false /* after */);
+        }
         if (testResult.getStatus() == ITestResult.SKIP) {
           Throwable t = testResult.getThrowable();
           if (t != null) {

testng-core/src/test/java/test/configuration/ConfigurationTest.java

@@ -5,6 +5,7 @@
 import java.util.Arrays;
 import org.testng.Assert;
 import org.testng.TestNG;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import org.testng.xml.XmlSuite;
 import test.configuration.issue2726.TestClassSample;
@@ -13,6 +14,8 @@
 import test.configuration.sample.ExternalConfigurationClassSample;
 import test.configuration.sample.MethodCallOrderTestSample;
 import test.configuration.sample.SuiteTestSample;
+import test.listeners.issue2961.OnlyOnceConfigurationThatFailsTestSample;
+import test.listeners.issue2961.OnlyOnceConfigurationThatPassesTestSample;
 
 /**
  * Test @Configuration
@@ -53,4 +56,20 @@ public void testAfterClassCalledOnlyOnceForParallelTestMethods() {
     assertThat(TestClassSample.beforeLogs).hasSize(1);
     assertThat(TestClassSample.afterLogs).hasSize(1);
   }
+
+  @Test(description = "GITHUB-2961", dataProvider = "produceTestClasses")
+  public void ensureFirstTimeOnlyConfigsHaveProperTestStatuses(Class<?> clazz) {
+    TestNG testng = create(clazz);
+    testng.setVerbose(2);
+    testng.run();
+    assertThat(testng.getStatus()).isZero();
+  }
+
+  @DataProvider(name = "produceTestClasses")
+  public Object[][] produceTestClasses() {
+    return new Object[][] {
+      {OnlyOnceConfigurationThatFailsTestSample.class},
+      {OnlyOnceConfigurationThatPassesTestSample.class}
+    };
+  }
 }

testng-core/src/test/java/test/configuration/issue2961/OnlyOnceConfigurationTest.java

@@ -0,0 +1,24 @@
+package test.configuration.issue2961;
+
+import java.lang.reflect.Method;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class OnlyOnceConfigurationTest extends ParentTestClass {
+
+  @BeforeMethod(firstTimeOnly = true)
+  public void beforeMethodFirstTimeOnlyTestClass(Method method) {
+    System.out.println("First Time Only Test Class " + method);
+  }
+
+  @Test
+  public void test() {
+    System.out.println("Test passed");
+  }
+
+  @AfterMethod(lastTimeOnly = true)
+  public void afterMethodLastTimeOnly() {
+    System.out.println("Last Time Only");
+  }
+}

testng-core/src/test/java/test/configuration/issue2961/ParentTestClass.java

@@ -0,0 +1,11 @@
+package test.configuration.issue2961;
+
+import java.lang.reflect.Method;
+import org.testng.annotations.BeforeMethod;
+
+public class ParentTestClass {
+  @BeforeMethod(firstTimeOnly = true)
+  public void beforeMethodFirstTimeOnlyParent(Method method) {
+    System.out.println("First Time Only Parent " + method);
+  }
+}

testng-core/src/test/java/test/listeners/issue2961/OnlyOnceConfigurationThatFailsTestSample.java

@@ -0,0 +1,20 @@
+package test.listeners.issue2961;
+
+import java.lang.reflect.Method;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class OnlyOnceConfigurationThatFailsTestSample extends ParentTestClassSample {
+
+  @BeforeMethod(firstTimeOnly = true)
+  public void beforeMethodFirstTimeOnlyTestClass(Method method) {
+    throw new RuntimeException("Example");
+  }
+
+  @Test
+  public void test() {}
+
+  @AfterMethod(lastTimeOnly = true)
+  public void afterMethodLastTimeOnly() {}
+}

testng-core/src/test/java/test/listeners/issue2961/OnlyOnceConfigurationThatPassesTestSample.java

@@ -0,0 +1,18 @@
+package test.listeners.issue2961;
+
+import java.lang.reflect.Method;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class OnlyOnceConfigurationThatPassesTestSample extends ParentTestClassSample {
+
+  @BeforeMethod(firstTimeOnly = true)
+  public void beforeMethodFirstTimeOnlyTestClass(Method method) {}
+
+  @Test
+  public void test() {}
+
+  @AfterMethod(lastTimeOnly = true)
+  public void afterMethodLastTimeOnly() {}
+}

testng-core/src/test/java/test/listeners/issue2961/ParentTestClassSample.java

@@ -0,0 +1,10 @@
+package test.listeners.issue2961;
+
+import java.lang.reflect.Method;
+import org.testng.annotations.BeforeMethod;
+
+public class ParentTestClassSample {
+
+  @BeforeMethod(firstTimeOnly = true)
+  public void beforeMethodFirstTimeOnlyParent(Method method) {}
+}