Handle exceptions in emailable Reporter

Closes #3038

Author: krmahadevan

CHANGES.txt

@@ -1,5 +1,6 @@
 Current
 7.10.0
+Fixed: GITHUB-3038: java.lang.IllegalStateException: Results per method should NOT have been empty (Krishnan Mahadevan)
 Fixed: GITHUB-3022: Remove deprecated JUnit related support in TestNG (Krishnan Mahadevan)
 
 7.9.0

testng-core/src/main/java/org/testng/reporters/EmailableReporter2.java

@@ -750,7 +750,7 @@ protected List<ClassResult> groupResults(Set<ITestResult> results) {
           String className = result.getTestClass().getName();
           if (!previousClassName.equals(className)) {
             // Different class implies different method
-            if (!resultsPerMethod.isEmpty()) {
+            if (resultsPerMethod.isEmpty()) {
               throw new IllegalStateException("Results per method should NOT have been empty");
             }
             resultsPerClass.add(new MethodResult(resultsPerMethod));
@@ -764,7 +764,9 @@ protected List<ClassResult> groupResults(Set<ITestResult> results) {
           } else {
             String methodName = result.getMethod().getMethodName();
             if (!previousMethodName.equals(methodName)) {
-              assert !resultsPerMethod.isEmpty();
+              if (resultsPerMethod.isEmpty()) {
+                throw new IllegalStateException("Results per method should NOT have been empty");
+              }
               resultsPerClass.add(new MethodResult(resultsPerMethod));
               resultsPerMethod = Lists.newArrayList();
 
@@ -773,7 +775,7 @@ protected List<ClassResult> groupResults(Set<ITestResult> results) {
           }
           resultsPerMethod.add(result);
         }
-        if (!resultsPerMethod.isEmpty()) {
+        if (resultsPerMethod.isEmpty()) {
           throw new IllegalStateException("Results per method should NOT have been empty");
         }
         resultsPerClass.add(new MethodResult(resultsPerMethod));

testng-core/src/test/java/test/reports/EmailableReporterTest.java

@@ -11,6 +11,10 @@
 import org.testng.annotations.Test;
 import org.testng.reporters.EmailableReporter2;
 import test.SimpleBaseTest;
+import test.reports.issue3038.AnotherTestCaseSample;
+import test.reports.issue3038.ExceptionAwareEmailableReporter;
+import test.reports.issue3038.TestCaseSample;
+import test.reports.issue3038.TestCaseWithConfigProblemSample;
 
 public class EmailableReporterTest extends SimpleBaseTest {
   @Test(dataProvider = "getReporterInstances", priority = 1)
@@ -35,6 +39,37 @@ public void testReportsNameCustomizationViaMainMethodInvocationAndJVMArguments(
     runTestViaMainMethod(clazzName, jvm);
   }
 
+  @Test
+  public void ensureEmailableReportsDontThrowExceptions() {
+    TestNG testng = create(TestCaseSample.class);
+    ExceptionAwareEmailableReporter reporter = new ExceptionAwareEmailableReporter();
+    testng.addListener(reporter);
+    testng.run();
+    assertThat(reporter.hasError).isFalse();
+  }
+
+  @Test
+  public void ensureEmailableReportsDontThrowExceptionsWhenMultipleClassesAreUsed() {
+    TestNG testng = create(TestCaseSample.class, AnotherTestCaseSample.class);
+    ExceptionAwareEmailableReporter reporter = new ExceptionAwareEmailableReporter();
+    testng.addListener(reporter);
+    testng.run();
+    assertThat(reporter.hasError).isFalse();
+  }
+
+  @Test
+  public void ensureEmailableReportsDontThrowExceptionsWhenConfigsHaveErrors() {
+    TestNG testng =
+        create(
+            TestCaseSample.class,
+            TestCaseWithConfigProblemSample.class,
+            AnotherTestCaseSample.class);
+    ExceptionAwareEmailableReporter reporter = new ExceptionAwareEmailableReporter();
+    testng.addListener(reporter);
+    testng.run();
+    assertThat(reporter.hasError).isFalse();
+  }
+
   @DataProvider(name = "getReporterInstances")
   public Object[][] getReporterInstances(Method method) {
     if (method.getName().toLowerCase().contains("jvmarguments")) {

testng-core/src/test/java/test/reports/issue3038/AnotherTestCaseSample.java

@@ -0,0 +1,13 @@
+package test.reports.issue3038;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class AnotherTestCaseSample {
+
+  @BeforeMethod
+  public void beforeMethod() {}
+
+  @Test
+  public void testHello() {}
+}

testng-core/src/test/java/test/reports/issue3038/ExceptionAwareEmailableReporter.java

@@ -0,0 +1,22 @@
+package test.reports.issue3038;
+
+import java.util.List;
+import org.testng.ISuite;
+import org.testng.reporters.EmailableReporter2;
+import org.testng.xml.XmlSuite;
+
+public class ExceptionAwareEmailableReporter extends EmailableReporter2 {
+
+  public boolean hasError = false;
+
+  @Override
+  public void generateReport(
+      List<XmlSuite> xmlSuites, List<ISuite> suites, String outputDirectory) {
+    try {
+      super.generateReport(xmlSuites, suites, outputDirectory);
+    } catch (IllegalStateException e) {
+      hasError = true;
+      throw e;
+    }
+  }
+}

testng-core/src/test/java/test/reports/issue3038/TestCaseSample.java

@@ -0,0 +1,13 @@
+package test.reports.issue3038;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class TestCaseSample {
+
+  @BeforeMethod
+  public void beforeMethod() {}
+
+  @Test
+  public void testHello() {}
+}

testng-core/src/test/java/test/reports/issue3038/TestCaseWithConfigProblemSample.java

@@ -0,0 +1,15 @@
+package test.reports.issue3038;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class TestCaseWithConfigProblemSample {
+
+  @BeforeMethod
+  public void beforeMethod() {
+    throw new RuntimeException("simulating a failure");
+  }
+
+  @Test
+  public void testHello() {}
+}