Include all data driven tests for reporting skips

Closes #2674

We now have a new configuration parameter named
“-includeAllDataDrivenTestsWhenSkipping” which 
when used would cause a data driven test to report
All its iterations as skipped when there is a failure
in its upstream dependencies.

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-2674: Run onTestSkipped for each value from data provider (Krishnan Mahadevan)
 Fixed: GITHUB-2672: Log real stacktrace when test times out. (cdalexndr)
 Fixed: GITHUB-2669: A failed retry with ITestContext will lose the ITestContext. (Nan Liang)
 Fixed: GITHUB-2643: assertEquals(Set,Set) now ignores ordering as it did before. (Elis Edlund)

testng-core/src/main/java/org/testng/CommandLineArgs.java

@@ -242,4 +242,13 @@ public class CommandLineArgs {
       description =
           "Comma separated fully qualified class names of listeners that should be skipped from being wired in via Service Loaders.")
   public Boolean overrideIncludedMethods = false;
+
+  public static final String INCLUDE_ALL_DATA_DRIVEN_TESTS_WHEN_SKIPPING =
+      "-includeAllDataDrivenTestsWhenSkipping";
+
+  @Parameter(
+      names = INCLUDE_ALL_DATA_DRIVEN_TESTS_WHEN_SKIPPING,
+      description =
+          "Should TestNG report all iterations of a data driven test as individual skips, in-case of upstream failures.")
+  public Boolean includeAllDataDrivenTestsWhenSkipping = false;
 }

testng-core/src/main/java/org/testng/TestNG.java

@@ -582,6 +582,14 @@ public void addMethodSelector(XmlMethodSelector selector) {
     m_selectors.add(selector);
   }
 
+  public void setReportAllDataDrivenTestsAsSkipped(boolean reportAllDataDrivenTestsAsSkipped) {
+    this.m_configuration.setReportAllDataDrivenTestsAsSkipped(reportAllDataDrivenTestsAsSkipped);
+  }
+
+  public boolean getReportAllDataDrivenTestsAsSkipped() {
+    return this.m_configuration.getReportAllDataDrivenTestsAsSkipped();
+  }
+
   /**
    * Set the suites file names to be run by this TestNG object. This method tries to load and parse
    * the specified TestNG suite xml files. If a file is missing, it is ignored.
@@ -1404,6 +1412,7 @@ public static TestNG privateMain(String[] argv, ITestListener listener) {
    * @param cla The command line parameters
    */
   protected void configure(CommandLineArgs cla) {
+    setReportAllDataDrivenTestsAsSkipped(cla.includeAllDataDrivenTestsWhenSkipping);
     if (cla.verbose != null) {
       setVerbose(cla.verbose);
     }
@@ -1613,6 +1622,10 @@ public void configure(Map cmdLineArgs) {
     result.xmlPathInJar = (String) cmdLineArgs.get(CommandLineArgs.XML_PATH_IN_JAR);
     result.junit = (Boolean) cmdLineArgs.get(CommandLineArgs.JUNIT);
     result.mixed = (Boolean) cmdLineArgs.get(CommandLineArgs.MIXED);
+    Object tmpValue = cmdLineArgs.get(CommandLineArgs.INCLUDE_ALL_DATA_DRIVEN_TESTS_WHEN_SKIPPING);
+    if (tmpValue != null) {
+      result.includeAllDataDrivenTestsWhenSkipping = Boolean.parseBoolean(tmpValue.toString());
+    }
     result.skipFailedInvocationCounts =
         (Boolean) cmdLineArgs.get(CommandLineArgs.SKIP_FAILED_INVOCATION_COUNTS);
     result.failIfAllTestsSkipped =

testng-core/src/main/java/org/testng/internal/Configuration.java

@@ -33,6 +33,8 @@ public class Configuration implements IConfiguration {
   private IInjectorFactory injectorFactory = new GuiceBackedInjectorFactory();
   private boolean overrideIncludedMethods = false;
 
+  private boolean includeAllDataDrivenTestsWhenSkipping;
+
   public Configuration() {
     init(new JDK15AnnotationFinder(new DefaultAnnotationTransformer()));
   }
@@ -144,4 +146,14 @@ public boolean getOverrideIncludedMethods() {
   public void setOverrideIncludedMethods(boolean overrideIncludedMethods) {
     this.overrideIncludedMethods = overrideIncludedMethods;
   }
+
+  @Override
+  public void setReportAllDataDrivenTestsAsSkipped(boolean reportAllDataDrivenTestsAsSkipped) {
+    this.includeAllDataDrivenTestsWhenSkipping = reportAllDataDrivenTestsAsSkipped;
+  }
+
+  @Override
+  public boolean getReportAllDataDrivenTestsAsSkipped() {
+    return this.includeAllDataDrivenTestsWhenSkipping;
+  }
 }

testng-core/src/main/java/org/testng/internal/IConfiguration.java

@@ -49,4 +49,10 @@ default boolean addExecutionListenerIfAbsent(IExecutionListener l) {
   boolean getOverrideIncludedMethods();
 
   void setOverrideIncludedMethods(boolean overrideIncludedMethods);
+
+  default void setReportAllDataDrivenTestsAsSkipped(boolean reportAllDataDrivenTestsAsSkipped) {}
+
+  default boolean getReportAllDataDrivenTestsAsSkipped() {
+    return false;
+  }
 }

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

@@ -7,6 +7,7 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
@@ -16,6 +17,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import org.testng.DataProviderHolder;
 import org.testng.DataProviderInvocationException;
@@ -37,6 +39,7 @@
 import org.testng.SuiteRunner;
 import org.testng.TestException;
 import org.testng.TestNGException;
+import org.testng.collections.CollectionUtils;
 import org.testng.collections.Lists;
 import org.testng.collections.Maps;
 import org.testng.collections.Sets;
@@ -104,12 +107,48 @@ public List<ITestResult> invokeTestMethods(
       //
       // Not okToProceed. Test is being skipped
       //
-      ITestResult result =
-          registerSkippedTestResult(
-              testMethod, System.currentTimeMillis(), new Throwable(okToProceed));
-      m_notifier.addSkippedTest(testMethod, result);
-      InvokedMethod invokedMethod = new InvokedMethod(System.currentTimeMillis(), result);
-      invokeListenersForSkippedTestResult(result, invokedMethod);
+      List<ITestResult> results = new ArrayList<>();
+      Consumer<ITestResult> resultProcessor =
+          result -> {
+            m_notifier.addSkippedTest(testMethod, result);
+            InvokedMethod invokedMethod = new InvokedMethod(System.currentTimeMillis(), result);
+            invokeListenersForSkippedTestResult(result, invokedMethod);
+          };
+      boolean reportAllDataDrivenTestsAsSkipped =
+          m_configuration.getReportAllDataDrivenTestsAsSkipped();
+      if (reportAllDataDrivenTestsAsSkipped && testMethod.isDataDriven()) {
+        ParameterHandler handler =
+            new ParameterHandler(
+                m_configuration.getObjectFactory(),
+                annotationFinder(),
+                buildDataProviderHolder(),
+                1);
+
+        ParameterBag bag =
+            handler.createParameters(
+                testMethod, Maps.newHashMap(), Maps.newHashMap(), context, instance);
+        Iterator<Object[]> allParamValues = Objects.requireNonNull(bag.parameterHolder).parameters;
+        Iterable<Object[]> allParameterValues = CollectionUtils.asIterable(allParamValues);
+        for (Object[] next : allParameterValues) {
+          if (next == null) {
+            continue;
+          }
+          Method m = testMethod.getConstructorOrMethod().getMethod();
+          Object[] parameterValues = Parameters.injectParameters(next, m, context);
+          ITestResult result =
+              registerSkippedTestResult(
+                  testMethod, System.currentTimeMillis(), new Throwable(okToProceed));
+          result.setParameters(parameterValues);
+          resultProcessor.accept(result);
+          results.add(result);
+        }
+      } else {
+        ITestResult result =
+            registerSkippedTestResult(
+                testMethod, System.currentTimeMillis(), new Throwable(okToProceed));
+        resultProcessor.accept(result);
+        results.add(result);
+      }
       testMethod.incrementCurrentInvocationCount();
       GroupConfigMethodArguments args =
           new Builder()
@@ -119,7 +158,7 @@ public List<ITestResult> invokeTestMethods(
               .withParameters(parameters)
               .build();
       this.invoker.invokeAfterGroupsConfigurations(args);
-      return Collections.singletonList(result);
+      return Collections.unmodifiableList(results);
     }
 
     // For invocationCount > 1 and threadPoolSize > 1 run this method in its own pool thread.

testng-core/src/test/java/test/skip/ReasonForSkipTest.java

@@ -5,13 +5,16 @@
 
 import java.util.Arrays;
 import java.util.Map;
+import org.testng.CommandLineArgs;
 import org.testng.ITestResult;
 import org.testng.TestNG;
 import org.testng.annotations.Test;
 import org.testng.collections.Maps;
 import org.testng.xml.XmlSuite;
+import test.InvokedMethodNameListener;
 import test.SimpleBaseTest;
 import test.skip.github1967.TestClassSample;
+import test.skip.issue2674.ConfigAwareTestNG;
 
 public class ReasonForSkipTest extends SimpleBaseTest {
 
@@ -103,6 +106,31 @@ public void testEnsureTestStatusIsSetProperlyForSkippedTests() {
     assertThat(actual).containsAllEntriesOf(expected);
   }
 
+  @Test(description = "GITHUB-2674")
+  public void ensureUpstreamFailuresTriggerSkipsForAllDataProviderValues() {
+    TestNG testng = create(test.skip.issue2674.TestClassSample.class);
+    testng.setReportAllDataDrivenTestsAsSkipped(true);
+    InvokedMethodNameListener listener = new InvokedMethodNameListener();
+    testng.addListener(listener);
+    testng.run();
+    assertThat(listener.getSkippedMethodNames())
+        .containsExactly("test2(iPhone,13)", "test2(iPhone-Pro,12)");
+  }
+
+  @Test(description = "GITHUB-2674")
+  public void ensureUpstreamFailuresTriggerSkipsForAllDataProviderValuesViaCmdLineArgs() {
+    CommandLineArgs cli = new CommandLineArgs();
+    cli.includeAllDataDrivenTestsWhenSkipping = true;
+    ConfigAwareTestNG testng = new ConfigAwareTestNG();
+    testng.setTestClasses(new Class<?>[] {test.skip.issue2674.TestClassSample.class});
+    testng.configure(cli);
+    InvokedMethodNameListener listener = new InvokedMethodNameListener();
+    testng.addListener(listener);
+    testng.run();
+    assertThat(listener.getSkippedMethodNames())
+        .containsExactly("test2(iPhone,13)", "test2(iPhone-Pro,12)");
+  }
+
   private static void runTest(Map<String, String> expected, Class<?>... clazz) {
     TestNG testng = create(clazz);
     ReasonReporter reporter = new ReasonReporter();

testng-core/src/test/java/test/skip/issue2674/ConfigAwareTestNG.java

@@ -0,0 +1,12 @@
+package test.skip.issue2674;
+
+import org.testng.CommandLineArgs;
+import org.testng.TestNG;
+
+public class ConfigAwareTestNG extends TestNG {
+
+  @Override
+  public void configure(CommandLineArgs cla) {
+    super.configure(cla);
+  }
+}

testng-core/src/test/java/test/skip/issue2674/TestClassSample.java

@@ -0,0 +1,22 @@
+package test.skip.issue2674;
+
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class TestClassSample {
+  @Test
+  void test1() {
+    Assert.fail();
+  }
+
+  @Test(
+      dataProvider = "items",
+      dependsOnMethods = {"test1"})
+  void test2(String model, int variant) {}
+
+  @DataProvider(name = "items")
+  Object[][] items() {
+    return new Object[][] {{"iPhone", 13}, {"iPhone-Pro", 12}};
+  }
+}