Merge pull request #50 from giurim/fix-beforeall-exception-handling

fix: Fix beforeAll exception handling and task reporting

Author: giurim

gradle-example/src/test/scala/co/helmethair/scalatest/example/CapitalNameTest.scala

@@ -0,0 +1,12 @@
+package co.helmethair.scalatest.example
+
+import org.scalatest.funspec.AnyFunSpec
+import org.scalatest.matchers.should.Matchers
+
+class CapitalNameTest extends AnyFunSpec with Matchers {
+  describe("Capital name") {
+    it("runs") {
+      true shouldBe true
+    }
+  }
+}

src/main/java/co/helmethair/scalatest/runtime/Executor.java

@@ -74,25 +74,29 @@ private void executeSuite(TestDescriptor test, JUnitReporter reporter) {
         List<ScalatestTestDescriptor> tests = children.stream().filter(c -> c instanceof ScalatestTestDescriptor)
                 .map(c -> (ScalatestTestDescriptor) c).collect(Collectors.toList());
 
-        Set<TestDescriptor> nonTests = new HashSet<>(children);
-        nonTests.removeAll(tests);
+        Set<TestDescriptor> subSuites = new HashSet<>(children);
+        subSuites.removeAll(tests);
 
+        subSuites.stream()
+                .sorted(Comparator.comparing(TestDescriptor::getDisplayName))
+                .forEach(c -> executeTest(c, reporter));
+
+        boolean suitExecutedOk = true;
         if (!tests.isEmpty()) {
-            runScalatests((ScalatestSuiteDescriptor) test,
+            suitExecutedOk = runScalatests((ScalatestSuiteDescriptor) test,
                     tests.stream()
                             .sorted(Comparator.comparing(TestDescriptor::getDisplayName))
                             .collect(Collectors.toList()),
                     reporter);
         }
 
-        nonTests.stream()
-                .sorted(Comparator.comparing(TestDescriptor::getDisplayName))
-                .forEach(c -> executeTest(c, reporter));
-
-        reporter.getJunitListener().executionFinished(test, TestExecutionResult.successful());
+        if (suitExecutedOk){
+            // if exception is thrown during suit execution (init, before/after all) we should not report a SUCCESS
+            reporter.getJunitListener().executionFinished(test, TestExecutionResult.successful());
+        }
     }
 
-    private void runScalatests(ScalatestSuiteDescriptor containingSuite, List<ScalatestTestDescriptor> tests, JUnitReporter reporter) {
+    private boolean runScalatests(ScalatestSuiteDescriptor containingSuite, List<ScalatestTestDescriptor> tests, JUnitReporter reporter) {
 
         Suite scalasuite = containingSuite.getScalasuite();
 
@@ -125,6 +129,7 @@ private void runScalatests(ScalatestSuiteDescriptor containingSuite, List<Scalat
         try {
             Status status = scalasuite.run(Option.apply(null), args);
             status.waitUntilCompleted();
+            return true;
         } catch (Throwable e) {
             if (e instanceof InstantiationException || e instanceof IllegalAccessException) {
                 reporter.apply(suiteAborted(args.tracker().nextOrdinal(), e, Resources.cannotInstantiateSuite(e.getMessage()), scalasuite));
@@ -139,6 +144,7 @@ private void runScalatests(ScalatestSuiteDescriptor containingSuite, List<Scalat
                     throw e;
                 }
             }
+            return false;
         }
     }
 

src/test/java/co/helmethair/scalatest/BeforeAfterTest.java

@@ -6,6 +6,7 @@
 import org.junit.platform.engine.EngineDiscoveryRequest;
 import org.junit.platform.engine.ExecutionRequest;
 import org.junit.platform.engine.TestDescriptor;
+import org.junit.platform.engine.TestExecutionResult;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -61,6 +62,7 @@ void beforeFailedTest() {
         verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
 
         verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInBeforeTest]", listener, null);
+        verifyTestSuccessNotReported("[engine:scalatest]/[suite:tests.FailInBeforeTest]", listener);
     }
 
     @Test
@@ -78,6 +80,7 @@ void beforeAllFailedTest() {
 
         verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
         verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInBeforeAllTest]", listener, null);
+        verifyTestSuccessNotReported("[engine:scalatest]/[suite:tests.FailInBeforeAllTest]", listener);
     }
 
     @Test
@@ -96,6 +99,7 @@ void afterFailedTest() {
 
         verifyTestSuccessReported("[engine:scalatest]/[suite:tests.FailInAfterTest]/[test:test]", listener);
         verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInAfterTest]", listener, null);
+        verifyTestSuccessNotReported("[engine:scalatest]/[suite:tests.FailInAfterTest]", listener);
     }
 
     @Test
@@ -116,5 +120,6 @@ void afterAllFailedTest() {
         verifyTestSuccessReported("[engine:scalatest]/[suite:tests.FailInAfterAllTest]/[test:test 1]", listener);
         verifyTestSuccessReported("[engine:scalatest]/[suite:tests.FailInAfterAllTest]/[test:test 2]", listener);
         verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInAfterAllTest]", listener, null);
+        verifyTestSuccessNotReported("[engine:scalatest]/[suite:tests.FailInAfterAllTest]", listener);
     }
 }

src/test/java/co/helmethair/scalatest/helper/TestHelpers.java

@@ -186,6 +186,15 @@ default void verifyTestSuccessReported(String testIdsuffix, TestEngineExecutionL
         );
     }
 
+    default void verifyTestSuccessNotReported(String testIdsuffix, TestEngineExecutionListener listener) {
+        verify(listener, never()).executionFinished(
+                argThat(a -> a.getUniqueId().toString().endsWith(testIdsuffix)),
+                argThat(a -> !a.getThrowable().isPresent()
+                        && a.getStatus() == TestExecutionResult.Status.SUCCESSFUL
+                )
+        );
+    }
+
     default void verifyTestSuccessReported(String testIdsuffix, TestExecutionListener listener) {
         verify(listener, atLeastOnce()).executionFinished(
                 argThat(a -> a.getUniqueId().endsWith(testIdsuffix)),