Skip config listener calls for skipped configs

Closes #2880 
Ensure that before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip'

Author: bobshie

CHANGES.txt

@@ -7,6 +7,7 @@ Fixed: GITHUB-2857: XmlTest index is not set for test suites invoked with YAML
 Fixed: GITHUB-2862: Allow test classes to define "configfailurepolicy" at a per class level (Krishnan Mahadevan)
 Fixed: GITHUB-2796: Option for onAfterClass to run after @AfterClass (Oliver Hughes)
 Fixed: GITHUB-2857: XmlTest index is not set for test suites invoked with YAML (Sergei Baranov)
+Fixed: GITHUB-2880: Before configuration and before invocation set 'SKIP' when beforeMethod is 'skip' (Bob Shi)
 
 7.7.1
 Fixed: GITHUB-2854: overloaded assertEquals methods do not work from Groovy (Krishnan Mahadevan)

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

@@ -279,10 +279,12 @@ public void invokeConfigurations(ConfigMethodArguments arguments) {
             && !alwaysRun) {
           log(3, "Skipping " + Utils.detailedMethodName(tm, true));
           InvokedMethod invokedMethod = new InvokedMethod(System.currentTimeMillis(), testResult);
+          // Set test result as 'SKIP' in 'beforeConfiguration' & 'beforeInvocation' if
+          // config method is skip.
+          testResult.setStatus(ITestResult.SKIP);
           runConfigurationListeners(testResult, arguments.getTestMethod(), true /* before */);
           runInvokedMethodListeners(BEFORE_INVOCATION, invokedMethod, testResult);
           testResult.setEndMillis(testResult.getStartMillis());
-          testResult.setStatus(ITestResult.SKIP);
           runInvokedMethodListeners(AFTER_INVOCATION, invokedMethod, testResult);
 
           handleConfigurationSkip(

testng-core/src/test/java/test/listeners/ListenersTest.java

@@ -25,6 +25,9 @@
 import test.listeners.issue2752.ListenerSample;
 import test.listeners.issue2752.TestClassSample;
 import test.listeners.issue2771.TestCaseSample;
+import test.listeners.issue2880.ListenerForIssue2880;
+import test.listeners.issue2880.TestClassWithFailingConfigsSample;
+import test.listeners.issue2880.TestClassWithPassingConfigsSample;
 
 public class ListenersTest extends SimpleBaseTest {
 
@@ -101,6 +104,17 @@ public void testEnsureNativeListenersAreRunAlwaysAtEnd() {
     assertThat(testng.getStatus()).isEqualTo(ExitCode.FAILED);
   }
 
+  @Test(description = "GITHUB-2880", dataProvider = "issue2880-dataprovider")
+  public void testSkipStatusInBeforeAndAfterConfigurationMethod(
+      Class<?> clazz, XmlSuite.FailurePolicy policy, List<String> expected) {
+    TestNG tng = create(clazz);
+    ListenerForIssue2880 listener = new ListenerForIssue2880();
+    tng.setConfigFailurePolicy(policy);
+    tng.addListener(listener);
+    tng.run();
+    assertThat(listener.getLogs()).containsExactlyElementsOf(expected);
+  }
+
   private void setupTest(boolean addExplicitListener) {
     TestNG testng = new TestNG();
     XmlSuite xmlSuite = createXmlSuite("Xml_Suite");
@@ -200,4 +214,53 @@ private static XmlSuite getNestedSuitesViaApis() {
     innerSuite2.setParentSuite(containerSuite);
     return containerSuite;
   }
+
+  @DataProvider(name = "issue2880-dataprovider")
+  public Object[][] getIssue2880Data() {
+    List<String> passList =
+        Arrays.asList(
+            "BeforeInvocation_beforeClass_STARTED",
+            "AfterInvocation_beforeClass_SUCCESS",
+            "BeforeInvocation_beforeMethod_STARTED",
+            "AfterInvocation_beforeMethod_SUCCESS",
+            "BeforeInvocation_testMethod_STARTED",
+            "AfterInvocation_testMethod_SUCCESS",
+            "BeforeInvocation_afterMethod_STARTED",
+            "AfterInvocation_afterMethod_SUCCESS",
+            "BeforeInvocation_afterClass_STARTED",
+            "AfterInvocation_afterClass_SUCCESS");
+
+    List<String> skipList =
+        Arrays.asList(
+            "BeforeInvocation_beforeClass_STARTED",
+            "AfterInvocation_beforeClass_FAILURE",
+            "BeforeInvocation_beforeMethod_SKIP",
+            "AfterInvocation_beforeMethod_SKIP",
+            "BeforeInvocation_testMethod_SKIP",
+            "AfterInvocation_testMethod_SKIP",
+            "BeforeInvocation_afterMethod_SKIP",
+            "AfterInvocation_afterMethod_SKIP",
+            "BeforeInvocation_afterClass_SKIP",
+            "AfterInvocation_afterClass_SKIP");
+
+    List<String> failList =
+        Arrays.asList(
+            "BeforeInvocation_beforeClass_STARTED",
+            "AfterInvocation_beforeClass_FAILURE",
+            "BeforeInvocation_beforeMethod_SKIP",
+            "AfterInvocation_beforeMethod_SKIP",
+            "BeforeInvocation_testMethod_SKIP",
+            "AfterInvocation_testMethod_SKIP",
+            "BeforeInvocation_afterMethod_SKIP",
+            "AfterInvocation_afterMethod_SKIP",
+            "BeforeInvocation_afterClass_SKIP",
+            "AfterInvocation_afterClass_SKIP");
+
+    return new Object[][] {
+      {TestClassWithPassingConfigsSample.class, XmlSuite.FailurePolicy.SKIP, passList},
+      {TestClassWithFailingConfigsSample.class, XmlSuite.FailurePolicy.SKIP, skipList},
+      {TestClassWithPassingConfigsSample.class, XmlSuite.FailurePolicy.CONTINUE, passList},
+      {TestClassWithFailingConfigsSample.class, XmlSuite.FailurePolicy.CONTINUE, failList}
+    };
+  }
 }

testng-core/src/test/java/test/listeners/github1602/IssueTest.java

@@ -34,17 +34,22 @@ public Object[][] getData() {
             "AfterInvocation_testMethod_SUCCESS",
             "BeforeInvocation_afterMethod_STARTED",
             "AfterInvocation_afterMethod_SUCCESS");
+
     List<String> baseList =
         Arrays.asList(
             "BeforeInvocation_beforeMethod_STARTED",
             "AfterInvocation_beforeMethod_FAILURE",
             "BeforeInvocation_testMethod_SKIP",
-            "AfterInvocation_testMethod_SKIP",
-            "BeforeInvocation_afterMethod_STARTED");
+            "AfterInvocation_testMethod_SKIP");
+
     List<String> skipList = Lists.newArrayList(baseList);
+    skipList.add("BeforeInvocation_afterMethod_SKIP");
     skipList.add("AfterInvocation_afterMethod_SKIP");
+
     List<String> failList = Lists.newArrayList(baseList);
+    failList.add("BeforeInvocation_afterMethod_STARTED");
     failList.add("AfterInvocation_afterMethod_FAILURE");
+
     return new Object[][] {
       {TestClassWithPassingConfigsSample.class, XmlSuite.FailurePolicy.SKIP, passList},
       {TestClassWithFailingConfigsSample.class, XmlSuite.FailurePolicy.SKIP, skipList},

testng-core/src/test/java/test/listeners/issue2880/ListenerForIssue2880.java

@@ -0,0 +1,53 @@
+package test.listeners.issue2880;
+
+import java.util.Collections;
+import java.util.List;
+import org.testng.IInvokedMethod;
+import org.testng.IInvokedMethodListener;
+import org.testng.ITestResult;
+import org.testng.collections.Lists;
+
+public class ListenerForIssue2880 implements IInvokedMethodListener {
+  private List<String> logs = Collections.synchronizedList(Lists.newArrayList());
+
+  @Override
+  public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
+    logs.add(
+        "BeforeInvocation_"
+            + method.getTestMethod().getMethodName()
+            + "_"
+            + intToStatus(testResult));
+  }
+
+  @Override
+  public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
+    logs.add(
+        "AfterInvocation_"
+            + method.getTestMethod().getMethodName()
+            + "_"
+            + intToStatus(testResult));
+  }
+
+  public List<String> getLogs() {
+    return logs;
+  }
+
+  private String intToStatus(ITestResult testResult) {
+    int status = testResult.getStatus();
+    switch (status) {
+      case ITestResult.CREATED:
+        return "CREATED";
+      case ITestResult.SUCCESS:
+        return "SUCCESS";
+      case ITestResult.SKIP:
+        return "SKIP";
+      case ITestResult.FAILURE:
+        return "FAILURE";
+      case ITestResult.STARTED:
+        return "STARTED";
+      case ITestResult.SUCCESS_PERCENTAGE_FAILURE:
+        return "SUCCESS_PERCENTAGE_FAILURE";
+    }
+    return " ??? " + String.valueOf(status);
+  }
+}

testng-core/src/test/java/test/listeners/issue2880/TestClassWithFailingConfigsSample.java

@@ -0,0 +1,26 @@
+package test.listeners.issue2880;
+
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class TestClassWithFailingConfigsSample {
+  @BeforeClass
+  public void beforeClass() {
+    throw new RuntimeException();
+  }
+
+  @BeforeMethod
+  public void beforeMethod() {}
+
+  @Test
+  public void testMethod() {}
+
+  @AfterMethod
+  public void afterMethod() {}
+
+  @AfterClass
+  public void afterClass() {}
+}

testng-core/src/test/java/test/listeners/issue2880/TestClassWithPassingConfigsSample.java

@@ -0,0 +1,25 @@
+package test.listeners.issue2880;
+
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class TestClassWithPassingConfigsSample {
+
+  @BeforeClass
+  public void beforeClass() {}
+
+  @BeforeMethod
+  public void beforeMethod() {}
+
+  @Test
+  public void testMethod() {}
+
+  @AfterMethod
+  public void afterMethod() {}
+
+  @AfterClass
+  public void afterClass() {}
+}