Support class level config failure policy

Closes #2862

CHANGES.txt

@@ -1,6 +1,7 @@
 Current
-Fixed: GITHUB-2796: Option for onAfterClass to run after @AfterClass
-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)
 
 7.7.1
 Fixed: GITHUB-2854: overloaded assertEquals methods do not work from Groovy (Krishnan Mahadevan)

testng-core-api/src/main/java/org/testng/annotations/DisregardConfigFailure.java

@@ -0,0 +1,18 @@
+package org.testng.annotations;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * A marker annotation that can be used to inform TestNG to ignore configuration failures for a
+ * specific class. This will have the last say in case the config failure policy was specified to be
+ * <code>continue</code> via the TestNG suite xml file.
+ *
+ * <p>This annotation can be added at a class level or at a method level.
+ */
+@Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
+@Target({METHOD, TYPE})
+public @interface DisregardConfigFailure {}

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

@@ -11,16 +11,19 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Stream;
 import org.testng.ConfigurationNotInvokedException;
 import org.testng.IClass;
 import org.testng.IConfigurable;
 import org.testng.IInvokedMethodListener;
+import org.testng.ITestClass;
 import org.testng.ITestContext;
 import org.testng.ITestNGMethod;
 import org.testng.ITestResult;
 import org.testng.Reporter;
 import org.testng.SuiteRunState;
 import org.testng.TestNGException;
+import org.testng.annotations.DisregardConfigFailure;
 import org.testng.annotations.IConfigurationAnnotation;
 import org.testng.collections.Maps;
 import org.testng.collections.Sets;
@@ -72,34 +75,49 @@ public ConfigInvoker(
    */
   public boolean hasConfigurationFailureFor(
       ITestNGMethod testNGMethod, String[] groups, IClass testClass, Object instance) {
+    return hasConfigurationFailureFor(null, testNGMethod, groups, testClass, instance);
+  }
+
+  @Override
+  public boolean hasConfigurationFailureFor(
+      ITestNGMethod configMethod,
+      ITestNGMethod testNGMethod,
+      String[] groups,
+      IClass testClass,
+      Object instance) {
     boolean result = false;
 
     Class<?> cls = testClass.getRealClass();
 
     if (m_suiteState.isFailed()) {
-      result = true;
-    } else {
-      boolean hasConfigurationFailures = classConfigurationFailed(cls, instance);
-      if (hasConfigurationFailures) {
-        if (!m_continueOnFailedConfiguration) {
-          result = true;
-        } else {
-          Set<Object> set = getInvocationResults(testClass);
-          result = set.contains(instance);
-        }
-        return result;
+      // if there were suite level failures, then short circuit here itself and report them.
+      return true;
+    }
+
+    boolean annotationFound = canDisregardConfigFailure(testClass);
+    boolean hasConfigurationFailures = classConfigurationFailed(cls, instance);
+    if (hasConfigurationFailures) {
+      if (annotationFound) {
+        // We were told to ignore failures via the annotation.
+        return false;
       }
-      // if method is BeforeClass, currentTestMethod will be null
-      if (m_continueOnFailedConfiguration && hasConfigFailure(testNGMethod)) {
-        Object key = TestNgMethodUtils.getMethodInvocationToken(testNGMethod, instance);
-        result = m_methodInvocationResults.get(testNGMethod).contains(key);
-      } else if (!m_continueOnFailedConfiguration) {
-        for (Class<?> clazz : m_classInvocationResults.keySet()) {
-          if (clazz.isAssignableFrom(cls)
-              && m_classInvocationResults.get(clazz).contains(instance)) {
-            result = true;
-            break;
-          }
+      if (m_continueOnFailedConfiguration) {
+        Set<Object> set = getInvocationResults(testClass);
+        result = set.contains(instance);
+      } else {
+        result = true;
+      }
+      return result;
+    }
+    // if method is BeforeClass, currentTestMethod will be null
+    if ((m_continueOnFailedConfiguration || annotationFound) && hasConfigFailure(testNGMethod)) {
+      Object key = TestNgMethodUtils.getMethodInvocationToken(testNGMethod, instance);
+      result = m_methodInvocationResults.get(testNGMethod).contains(key);
+    } else if (!(m_continueOnFailedConfiguration || annotationFound)) {
+      for (Class<?> clazz : m_classInvocationResults.keySet()) {
+        if (clazz.isAssignableFrom(cls) && m_classInvocationResults.get(clazz).contains(instance)) {
+          result = true;
+          break;
         }
       }
     }
@@ -245,6 +263,7 @@ public void invokeConfigurations(ConfigMethodArguments arguments) {
           continue;
         }
         if (hasConfigurationFailureFor(
+                tm,
                 arguments.getTestMethod(),
                 tm.getGroups(),
                 arguments.getTestClass(),
@@ -534,18 +553,18 @@ private void recordConfigurationInvocationFailed(
     if (annotation.getBeforeTestClass() || annotation.getAfterTestClass()) {
       // tm is the configuration method, and currentTestMethod is null for BeforeClass
       // methods, so we need testClass
-      if (m_continueOnFailedConfiguration) {
-        setClassInvocationFailure(testClass.getRealClass(), instance);
-      } else {
-        setClassInvocationFailure(tm.getRealClass(), instance);
+      Class<?> clazzToUse = tm.getRealClass();
+      if (m_continueOnFailedConfiguration || canDisregardConfigFailure(testClass.getRealClass())) {
+        clazzToUse = testClass.getRealClass();
       }
+      setClassInvocationFailure(clazzToUse, instance);
     }
 
     // If before/afterTestMethod failed, mark either the config method's entire
     // class as failed, or just the current test method as failed, depending on
     // the configuration failure policy
     else if (annotation.getBeforeTestMethod() || annotation.getAfterTestMethod()) {
-      if (m_continueOnFailedConfiguration) {
+      if (m_continueOnFailedConfiguration || canDisregardConfigFailure(tm)) {
         setMethodInvocationFailure(currentTestMethod, instance);
       } else {
         setClassInvocationFailure(tm.getRealClass(), instance);
@@ -606,4 +625,39 @@ private Set<Object> getInvocationResults(IClass testClass) {
     }
     return set;
   }
+
+  private static boolean canDisregardConfigFailure(Class<?> cls) {
+    while (!Object.class.equals(cls)) {
+      if (cls.isAnnotationPresent(DisregardConfigFailure.class)) {
+        return true;
+      }
+      cls = cls.getSuperclass();
+    }
+    return false;
+  }
+
+  private static boolean canDisregardConfigFailure(ITestNGMethod method) {
+    if (method == null) {
+      return false;
+    }
+    return method
+            .getConstructorOrMethod()
+            .getMethod()
+            .isAnnotationPresent(DisregardConfigFailure.class)
+        || canDisregardConfigFailure(method.getRealClass());
+  }
+
+  private static boolean canDisregardConfigFailure(IClass testClass) {
+    boolean instanceMatch = testClass instanceof ITestClass;
+    if (!instanceMatch) {
+      return false;
+    }
+    ITestClass tc = ((ITestClass) testClass);
+    return Stream.of(
+            tc.getBeforeTestConfigurationMethods() /*Considering @BeforeTest*/,
+            tc.getBeforeTestMethods() /*Considering @BeforeMethod*/,
+            tc.getBeforeClassMethods() /*Considering @BeforeClass*/)
+        .flatMap(Arrays::stream)
+        .anyMatch(ConfigInvoker::canDisregardConfigFailure);
+  }
 }

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

@@ -8,6 +8,13 @@ public interface IConfigInvoker {
   boolean hasConfigurationFailureFor(
       ITestNGMethod testNGMethod, String[] groups, IClass testClass, Object instance);
 
+  boolean hasConfigurationFailureFor(
+      ITestNGMethod configMethod,
+      ITestNGMethod testNGMethod,
+      String[] groups,
+      IClass testClass,
+      Object instance);
+
   void invokeBeforeGroupsConfigurations(GroupConfigMethodArguments arguments);
 
   void invokeAfterGroupsConfigurations(GroupConfigMethodArguments arguments);

testng-core/src/test/java/test/configurationfailurepolicy/FailurePolicyTest.java

@@ -1,18 +1,31 @@
 package test.configurationfailurepolicy;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.testng.Assert.assertEquals;
-import static test.SimpleBaseTest.getPathToResource;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import org.testng.ITestContext;
+import org.testng.ITestResult;
 import org.testng.TestListenerAdapter;
 import org.testng.TestNG;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.DataProvider;
+import org.testng.annotations.DisregardConfigFailure;
 import org.testng.annotations.Test;
 import org.testng.testhelper.OutputDirectoryPatch;
 import org.testng.xml.XmlSuite;
+import test.SimpleBaseTest;
+import test.configurationfailurepolicy.issue2862.AnnotationAtClassLevelForMethodConfigSample;
+import test.configurationfailurepolicy.issue2862.AnnotationAtMethodLevelForMethodConfigSample;
+import test.configurationfailurepolicy.issue2862.AnnotationAtParentClassLevelForMethodConfigSample;
+import test.configurationfailurepolicy.issue2862.AnnotationAtParentClassLevelForMethodConfigSample2;
+import test.configurationfailurepolicy.issue2862.AnnotationAtParentClassLevelForMethodConfigSample3;
+import test.configurationfailurepolicy.issue2862.ConfigFailurePolicyAwareReporter;
 
-public class FailurePolicyTest {
+public class FailurePolicyTest extends SimpleBaseTest {
 
   // only if this is run from an xml file that sets this on the suite
   @BeforeClass(enabled = false)
@@ -57,7 +70,7 @@ public Object[][] getData() {
 
   @Test(dataProvider = "dp")
   public void confFailureTest(
-      Class[] classesUnderTest,
+      Class<?>[] classesUnderTest,
       int configurationFailures,
       int configurationSkips,
       int skippedTests) {
@@ -75,7 +88,7 @@ public void confFailureTest(
 
   @Test
   public void confFailureTestInvolvingGroups() {
-    Class[] classesUnderTest =
+    Class<?>[] classesUnderTest =
         new Class[] {ClassWithFailedBeforeClassMethodAndBeforeGroupsAfterClassAfterGroups.class};
 
     TestListenerAdapter tla = new TestListenerAdapter();
@@ -178,4 +191,131 @@ private void verify(
         "wrong number of configuration skips");
     assertEquals(tla.getSkippedTests().size(), skippedTests, "wrong number of skipped tests");
   }
+
+  @Test(description = "GITHUB-2862")
+  public void ensureFailurePolicyCanBeOverriddenWithAnnotationAtMethodLevel() {
+    Class<?> clazzOne = AnnotationAtMethodLevelForMethodConfigSample.AnnotatedClassSample.class;
+    Class<?> clazzTwo = AnnotationAtMethodLevelForMethodConfigSample.RegularClassSample.class;
+    TestNG tng = create(clazzOne, clazzTwo);
+    ConfigFailurePolicyAwareReporter reporter = new ConfigFailurePolicyAwareReporter();
+    tng.addListener(reporter);
+    tng.run();
+    Map<Class<?>, Map<Integer, List<ITestResult>>> grouped = reporter.getGrouped();
+    assertThat(grouped.get(clazzOne).get(ITestResult.SKIP))
+        .withFailMessage(
+            clazzOne.getName()
+                + " should have ONLY 1 skipped test due to config failure since this "
+                + "class has the custom annotation @"
+                + DisregardConfigFailure.class.getSimpleName())
+        .hasSize(1);
+
+    assertThat(grouped.get(clazzTwo).get(ITestResult.SKIP))
+        .withFailMessage(
+            clazzTwo.getName()
+                + " should have 2 skipped test due to config failure since this "
+                + "class DOES NOT has the custom annotation @"
+                + DisregardConfigFailure.class.getSimpleName()
+                + " and no config failure policy has been set.")
+        .hasSize(2);
+  }
+
+  @Test(description = "GITHUB-2862")
+  public void ensureFailurePolicyCanBeOverriddenWithAnnotationAtClassLevel() {
+    Class<?> clazzOne = AnnotationAtClassLevelForMethodConfigSample.AnnotatedClassSample.class;
+    Class<?> clazzTwo = AnnotationAtClassLevelForMethodConfigSample.RegularClassSample.class;
+    TestNG tng = create(clazzOne, clazzTwo);
+    ConfigFailurePolicyAwareReporter reporter = new ConfigFailurePolicyAwareReporter();
+    tng.addListener(reporter);
+    tng.run();
+    Map<Class<?>, Map<Integer, List<ITestResult>>> grouped = reporter.getGrouped();
+    assertThat(grouped.get(clazzOne).get(ITestResult.SKIP))
+        .withFailMessage(
+            clazzOne.getName()
+                + " should have ONLY 1 skipped test due to config failure since this "
+                + "class has the custom annotation @"
+                + DisregardConfigFailure.class.getSimpleName())
+        .hasSize(1);
+
+    assertThat(grouped.get(clazzTwo).get(ITestResult.SKIP))
+        .withFailMessage(
+            clazzTwo.getName()
+                + " should have 2 skipped test due to config failure since this "
+                + "class DOES NOT has the custom annotation @"
+                + DisregardConfigFailure.class.getSimpleName()
+                + " and no config failure policy has been set.")
+        .hasSize(2);
+  }
+
+  @Test(description = "GITHUB-2862")
+  // Scenario: Annotation at base class, but no config methods in base class
+  public void ensureFailurePolicyCanBeOverriddenWithAnnotationAtBaseClassLevel() {
+    Class<?> clazzOne =
+        AnnotationAtParentClassLevelForMethodConfigSample.AnnotatedClassSample.class;
+    Class<?> clazzTwo = AnnotationAtParentClassLevelForMethodConfigSample.RegularClassSample.class;
+    TestNG tng = create(clazzOne, clazzTwo);
+    ConfigFailurePolicyAwareReporter reporter = new ConfigFailurePolicyAwareReporter();
+    tng.addListener(reporter);
+    tng.run();
+    Map<Class<?>, Map<Integer, List<ITestResult>>> grouped = reporter.getGrouped();
+    assertThat(grouped.get(clazzOne).get(ITestResult.SKIP))
+        .withFailMessage(
+            clazzOne.getName()
+                + " should have ONLY 1 skipped test due to config failure since this "
+                + "class has the custom annotation @"
+                + DisregardConfigFailure.class.getName())
+        .hasSize(1);
+
+    assertThat(grouped.get(clazzTwo).get(ITestResult.SKIP))
+        .withFailMessage(
+            clazzTwo.getSimpleName()
+                + " should have 2 skipped test due to config failure since this "
+                + "class DOES NOT has the custom annotation @"
+                + DisregardConfigFailure.class.getSimpleName()
+                + " and no config failure policy has been set.")
+        .hasSize(2);
+  }
+
+  @Test(description = "GITHUB-2862")
+  // Scenario: Annotation at base class, with a failing "@BeforeTest"
+  public void ensureFailurePolicyCanBeOverriddenWithAnnotationAndFailuresAtBaseClassLevel() {
+    Class<?> clazzOne =
+        AnnotationAtParentClassLevelForMethodConfigSample2.AnnotatedClassSample.class;
+    TestNG tng = create(clazzOne);
+    ConfigFailurePolicyAwareReporter reporter = new ConfigFailurePolicyAwareReporter();
+    tng.addListener(reporter);
+    tng.run();
+    Map<Class<?>, Map<Integer, List<ITestResult>>> grouped = reporter.getGrouped();
+    assertThat(
+            grouped
+                .getOrDefault(clazzOne, new HashMap<>())
+                .getOrDefault(ITestResult.SKIP, new ArrayList<>()))
+        .withFailMessage(
+            clazzOne.getName()
+                + " should NOT HAVE ANY skipped test due to config failure since this "
+                + "class has the custom annotation @"
+                + DisregardConfigFailure.class.getName())
+        .isEmpty();
+  }
+
+  @Test(description = "GITHUB-2862")
+  // Scenario: Annotation at base class, with a failing "@BeforeClass"
+  public void ensureFailurePolicyCanBeOverriddenWithAnnotationAndFailuresAtBaseClassLevel2() {
+    Class<?> clazzOne =
+        AnnotationAtParentClassLevelForMethodConfigSample3.AnnotatedClassSample.class;
+    TestNG tng = create(clazzOne);
+    ConfigFailurePolicyAwareReporter reporter = new ConfigFailurePolicyAwareReporter();
+    tng.addListener(reporter);
+    tng.run();
+    Map<Class<?>, Map<Integer, List<ITestResult>>> grouped = reporter.getGrouped();
+    assertThat(
+            grouped
+                .getOrDefault(clazzOne, new HashMap<>())
+                .getOrDefault(ITestResult.SKIP, new ArrayList<>()))
+        .withFailMessage(
+            clazzOne.getName()
+                + " should NOT HAVE ANY skipped test due to config failure since this "
+                + "class has the custom annotation @"
+                + DisregardConfigFailure.class.getName())
+        .isEmpty();
+  }
 }