Merge pull request #1579 from krmahadevan/krmahadevan-fix-217

Exception in DataProvider doesn't fail test run

Author: cbeust

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-217: exception in DataProvider doesn't fail test run (Krishnan Mahadevan)
 Fixed: GITHUB-987: Parameters threadCount and parallel doesn't work with maven (Krishnan Mahadevan)
 Fixed: GITHUB-1472: Optimize DynamicGraph.getUnfinishedNodes (Krishnan Mahadevan & Nathan Reynolds)
 Fixed: GITHUB-1566: Invalid XML characters in Params in testng-results.xml (Krishnan Mahadevan)

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

@@ -29,6 +29,7 @@
 import org.testng.internal.ClassHelper;
 import org.testng.internal.Configuration;
 import org.testng.internal.DynamicGraph;
+import org.testng.internal.ExitCode;
 import org.testng.internal.IConfiguration;
 import org.testng.internal.IResultListener2;
 import org.testng.internal.OverrideProcessor;
@@ -142,16 +143,9 @@ public class TestNG {
   private final Map<Class<? extends IReporter>, IReporter> m_reporters = Maps.newHashMap();
   private final Map<Class<? extends IDataProviderListener>, IDataProviderListener> m_dataProviderListeners = Maps.newHashMap();
 
-  protected static final int HAS_FAILURE = 1;
-  protected static final int HAS_SKIPPED = 2;
-  protected static final int HAS_FSP = 4;
-  protected static final int HAS_NO_TEST = 8;
 
   public static final Integer DEFAULT_VERBOSE = 1;
 
-  private int m_status;
-  private boolean m_hasTests= false;
-
   // Command line suite parameters
   private int m_threadCount = -1;
   private XmlSuite.ParallelMode m_parallelMode = null;
@@ -185,6 +179,8 @@ public class TestNG {
   private final Map<Class<? extends IAlterSuiteListener>, IAlterSuiteListener> m_alterSuiteListeners= Maps.newHashMap();
 
   private boolean m_isInitialized = false;
+  private org.testng.internal.ExitCodeListener exitCodeListener;
+  private ExitCode exitCode;
 
   /**
    * Default constructor. Setting also usage of default listeners/reporters.
@@ -211,11 +207,10 @@ private void init(boolean useDefaultListeners) {
   }
 
   public int getStatus() {
-    return m_status;
-  }
-
-  private void setStatus(int status) {
-    m_status |= status;
+    if (!exitCodeListener.hasTests()) {
+      return ExitCode.HAS_NO_TEST;
+    }
+    return exitCode.getExitCode();
   }
 
   /**
@@ -963,7 +958,8 @@ private void addReporter(Class<? extends IReporter> r) {
   }
 
   private void initializeDefaultListeners() {
-    addListener((ITestNGListener) new ExitCodeListener(this));
+    this.exitCodeListener = new org.testng.internal.ExitCodeListener();
+    addListener((ITestNGListener) this.exitCodeListener);
     if (m_useDefaultListeners) {
       addReporter(SuiteHTMLReporter.class);
       addReporter(Main.class);
@@ -1155,9 +1151,9 @@ public void run() {
     }
 
     runExecutionListeners(false /* finish */);
+    exitCode = this.exitCodeListener.getStatus();
 
-    if(!m_hasTests) {
-      setStatus(HAS_NO_TEST);
+    if(!exitCodeListener.hasTests()) {
       if (TestRunner.getVerbose() > 1) {
         System.err.println("[TestNG] No tests found. Nothing was run");
         usage();
@@ -1243,7 +1239,6 @@ private void generateReports(List<ISuite> suiteRunners) {
    */
   public List<ISuite> runSuitesLocally() {
     if (m_suites.isEmpty()) {
-      setStatus(HAS_NO_TEST);
       error("No test suite found. Nothing to run");
       usage();
       return Collections.emptyList();
@@ -1490,7 +1485,7 @@ public static TestNG privateMain(String[] argv, ITestListener listener) {
       else {
         error(ex.getMessage());
       }
-      result.setStatus(HAS_FAILURE);
+      result.exitCode = ExitCode.newExitCodeRepresentingFailure();
     }
 
     return result;
@@ -1844,21 +1839,21 @@ protected static void validateCommandLineParameters(CommandLineArgs args) {
    * @return true if at least one test failed.
    */
   public boolean hasFailure() {
-    return (getStatus() & HAS_FAILURE) == HAS_FAILURE;
+    return this.exitCode.hasFailure();
   }
 
   /**
    * @return true if at least one test failed within success percentage.
    */
   public boolean hasFailureWithinSuccessPercentage() {
-    return (getStatus() & HAS_FSP) == HAS_FSP;
+    return this.exitCode.hasFailureWithinSuccessPercentage();
   }
 
   /**
    * @return true if at least one test was skipped.
    */
   public boolean hasSkip() {
-    return (getStatus() & HAS_SKIPPED) == HAS_SKIPPED;
+    return this.exitCode.hasSkip();
   }
 
   static void exitWithError(String msg) {
@@ -1951,30 +1946,10 @@ public static TestNG getDefault() {
     return m_instance;
   }
 
-  /**
-   * @deprecated since 5.1
-   */
-  @Deprecated
-  public void setHasFailure(boolean hasFailure) {
-    m_status |= HAS_FAILURE;
-  }
-
-  /**
-   * @deprecated since 5.1
-   */
   @Deprecated
-  public void setHasFailureWithinSuccessPercentage(boolean hasFailureWithinSuccessPercentage) {
-    m_status |= HAS_FSP;
-  }
-
   /**
-   * @deprecated since 5.1
+   * @deprecated - This class stands deprecated as of TestNG v6.13
    */
-  @Deprecated
-  public void setHasSkip(boolean hasSkip) {
-    m_status |= HAS_SKIPPED;
-  }
-
   public static class ExitCodeListener implements IResultListener2 {
     private TestNG m_mainRunner;
 
@@ -1993,21 +1968,16 @@ public void beforeConfiguration(ITestResult tr) {
     @Override
     public void onTestFailure(ITestResult result) {
       setHasRunTests();
-      m_mainRunner.setStatus(HAS_FAILURE);
     }
 
     @Override
     public void onTestSkipped(ITestResult result) {
       setHasRunTests();
-      if ((m_mainRunner.getStatus() & HAS_FAILURE) != 0) {
-        m_mainRunner.setStatus(HAS_SKIPPED);
-      }
     }
 
     @Override
     public void onTestFailedButWithinSuccessPercentage(ITestResult result) {
       setHasRunTests();
-      m_mainRunner.setStatus(HAS_FSP);
     }
 
     @Override
@@ -2030,23 +2000,20 @@ public void onTestStart(ITestResult result) {
     }
 
     private void setHasRunTests() {
-      m_mainRunner.m_hasTests= true;
     }
 
     /**
      * @see org.testng.IConfigurationListener#onConfigurationFailure(org.testng.ITestResult)
      */
     @Override
     public void onConfigurationFailure(ITestResult itr) {
-      m_mainRunner.setStatus(HAS_FAILURE);
     }
 
     /**
      * @see org.testng.IConfigurationListener#onConfigurationSkip(org.testng.ITestResult)
      */
     @Override
     public void onConfigurationSkip(ITestResult itr) {
-      m_mainRunner.setStatus(HAS_SKIPPED);
     }
 
     /**

src/main/java/org/testng/TestNGAntTask.java

@@ -36,6 +36,7 @@
 import org.apache.tools.ant.types.resources.FileResource;
 import org.apache.tools.ant.types.selectors.FilenameSelector;
 import org.testng.collections.Lists;
+import org.testng.internal.ExitCode;
 import org.testng.internal.Utils;
 import org.testng.reporters.VerboseReporter;
 
@@ -721,10 +722,10 @@ private void ppp(String string) {
   protected void actOnResult(int exitValue, boolean wasKilled) {
     if(exitValue == -1) {
       executeHaltTarget(exitValue);
-      throw new BuildException("an error occured when running TestNG tests");
+      throw new BuildException("an error occurred when running TestNG tests");
     }
 
-    if((exitValue & TestNG.HAS_NO_TEST) == TestNG.HAS_NO_TEST) {
+    if((exitValue & ExitCode.HAS_NO_TEST) == ExitCode.HAS_NO_TEST) {
       if(m_haltOnFailure) {
         executeHaltTarget(exitValue);
         throw new BuildException("No tests were run");
@@ -738,7 +739,7 @@ protected void actOnResult(int exitValue, boolean wasKilled) {
       }
     }
 
-    boolean failed= ((exitValue & TestNG.HAS_FAILURE) == TestNG.HAS_FAILURE) || wasKilled;
+    boolean failed= (ExitCode.hasFailure(exitValue)) || wasKilled;
     if(failed) {
       final String msg= wasKilled ? "The tests timed out and were killed." : "The tests failed.";
       if(m_haltOnFailure) {
@@ -754,7 +755,7 @@ protected void actOnResult(int exitValue, boolean wasKilled) {
       }
     }
 
-    if((exitValue & TestNG.HAS_SKIPPED) == TestNG.HAS_SKIPPED) {
+    if(ExitCode.hasSkipped(exitValue)) {
       if(m_haltOnSkipped) {
         executeHaltTarget(exitValue);
         throw new BuildException("There are TestNG SKIPPED tests");
@@ -768,7 +769,7 @@ protected void actOnResult(int exitValue, boolean wasKilled) {
       }
     }
 
-    if((exitValue & TestNG.HAS_FSP) == TestNG.HAS_FSP) {
+    if(ExitCode.hasFailureWithinSuccessPercentage(exitValue)) {
       if(m_haltOnFSP) {
         executeHaltTarget(exitValue);
         throw new BuildException("There are TestNG FAILED WITHIN SUCCESS PERCENTAGE tests");

src/main/java/org/testng/internal/ExitCode.java

@@ -0,0 +1,99 @@
+package org.testng.internal;
+
+import org.testng.IResultMap;
+import org.testng.ITestContext;
+
+import java.util.BitSet;
+
+/**
+ * |---------------------|---------|--------|-------------|------------------------------------------|
+ * | FailedWithinSuccess | Skipped | Failed | Status Code | Remarks                                  |
+ * |---------------------|---------|--------|-------------|------------------------------------------|
+ * | 0                   | 0       | 0      | 0           | Passed tests                             |
+ * | 0                   | 0       | 1      | 1           | Failed tests                             |
+ * | 0                   | 1       | 0      | 2           | Skipped tests                            |
+ * | 0                   | 1       | 1      | 3           | Skipped/Failed tests                     |
+ * | 1                   | 0       | 0      | 4           | FailedWithinSuccess tests                |
+ * | 1                   | 0       | 1      | 5           | FailedWithinSuccess/Failed tests         |
+ * | 1                   | 1       | 0      | 6           | FailedWithinSuccess/Skipped tests        |
+ * | 1                   | 1       | 1      | 7           | FailedWithinSuccess/Skipped/Failed tests |
+ * |---------------------|---------|--------|-------------|------------------------------------------|
+ */
+public class ExitCode {
+
+    public static final int HAS_NO_TEST = 8;
+    private static final int FAILED_WITHIN_SUCCESS = 4;
+    private static final int SKIPPED = 2;
+    private static final int FAILED = 1;
+    private static final int SIZE = 3;
+
+    private final BitSet exitCodeBits;
+
+    ExitCode() {
+        this(new BitSet(SIZE));
+    }
+
+    public static boolean hasFailureWithinSuccessPercentage(int returnCode) {
+        return (returnCode & FAILED_WITHIN_SUCCESS) == FAILED_WITHIN_SUCCESS;
+    }
+
+    public static boolean hasSkipped(int returnCode) {
+        return (returnCode & SKIPPED) == SKIPPED;
+    }
+
+    public static boolean hasFailure(int returnCode) {
+        return (returnCode & FAILED) == FAILED;
+    }
+
+    public static ExitCode newExitCodeRepresentingFailure() {
+        BitSet bitSet = new BitSet(SIZE);
+        bitSet.set(0, true);
+        bitSet.set(1, false);
+        bitSet.set(2, false);
+
+        return new ExitCode(bitSet);
+    }
+
+    private ExitCode(BitSet exitCodeBits) {
+        this.exitCodeBits = exitCodeBits;
+    }
+
+    void computeAndUpdate(ITestContext context) {
+        computeAndUpdate(0, context.getFailedTests(), context.getFailedConfigurations());
+        computeAndUpdate(1, context.getSkippedTests(), context.getSkippedConfigurations());
+        computeAndUpdate(2, context.getFailedButWithinSuccessPercentageTests(), null);
+    }
+
+    private void computeAndUpdate(int index, IResultMap testResults, IResultMap configResults) {
+        boolean containsResults = testResults.size() != 0;
+        if (configResults != null) {
+            containsResults = containsResults || configResults.size() != 0;
+        }
+        if (containsResults) {
+            this.exitCodeBits.set(index);
+        }
+    }
+
+    public boolean hasFailure() {
+        return exitCodeBits.get(0);
+    }
+
+    public boolean hasSkip() {
+        return exitCodeBits.get(1);
+    }
+
+    public boolean hasFailureWithinSuccessPercentage() {
+        return exitCodeBits.get(2);
+    }
+
+    public int getExitCode() {
+        int exitCode = 0;
+        for (int i = 0; i < exitCodeBits.length(); i++) {
+            if (exitCodeBits.get(i)) {
+                exitCode = exitCode | (1 << i);
+            }
+        }
+
+        return exitCode;
+    }
+}