Merge pull request #23 from giurim/fail-when-beforeafter-fails

fix: report before/after failures and ingnored tests

src/main/java/co/helmethair/scalatest/reporter/JUnitReporter.java

@@ -7,12 +7,16 @@
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestExecutionResult;
 import org.scalatest.Reporter;
+import org.scalatest.Suite;
 import org.scalatest.events.*;
 import scala.Option;
+import scala.reflect.internal.Trees;
 
 import java.util.Collections;
 import java.util.Optional;
 
+import static co.helmethair.scalatest.scala.OptionHelper.getOrElse;
+
 public class JUnitReporter implements Reporter {
     private final EngineExecutionListener junitListener;
     private final TestDescriptor rootTestDescriptor;
@@ -30,58 +34,50 @@ public void apply(Event event) {
             TestStarting e = (TestStarting) event;
             junitListener.executionStarted(
                     getOrCreateDescriptor(e.suiteId(), e.suiteName(), e.testName()));
-        }
-        if (event instanceof TestCanceled) {
+        } else if (event instanceof TestCanceled) {
             TestCanceled e = (TestCanceled) event;
             junitListener.executionFinished(
                     getOrCreateDescriptor(e.suiteId(), e.suiteName(), e.testName()),
                     TestExecutionResult.aborted(getOrElse(e.throwable(), null)));
-        }
-        if (event instanceof TestSucceeded) {
+        } else if (event instanceof TestSucceeded) {
             TestSucceeded e = (TestSucceeded) event;
             junitListener.executionFinished(
                     getOrCreateDescriptor(e.suiteId(), e.suiteName(), e.testName()),
                     TestExecutionResult.successful());
-        }
-        if (event instanceof TestFailed) {
+        } else if (event instanceof TestFailed) {
             TestFailed e = (TestFailed) event;
             TestDescriptor descriptor = getOrCreateDescriptor(e.suiteId(), e.suiteName(), e.testName());
             Throwable cause = getOrElse(e.throwable(), null);
             setSkipWithCause(cause);
             junitListener.executionFinished(
                     descriptor,
                     TestExecutionResult.failed(cause));
-        }
-        if (event instanceof RunAborted) {
+        } else if (event instanceof RunAborted) {
             junitListener.executionFinished(rootTestDescriptor,
                     TestExecutionResult.aborted(getOrElse(((RunAborted) event).throwable(), null)));
-        }
-
-        if (event instanceof SuiteStarting) {
+        } else if (event instanceof SuiteStarting) {
             SuiteStarting e = (SuiteStarting) event;
             junitListener.executionStarted(
                     getOrCreateDescriptor(e.suiteId(), e.suiteName(), null));
-        }
-
-        if (event instanceof SuiteAborted) {
+        } else if (event instanceof SuiteAborted) {
             SuiteAborted e = (SuiteAborted) event;
+            Throwable ex = getOrElse(e.throwable(), null);
+            TestDescriptor suiteDescriptor = getOrCreateDescriptor(e.suiteId(), e.suiteName(), null);
             junitListener.executionFinished(
-                    getOrCreateDescriptor(e.suiteId(), e.suiteName(), null),
-                    TestExecutionResult.aborted(getOrElse(e.throwable(), null)));
-        }
-
-        if (event instanceof SuiteCompleted) {
+                    suiteDescriptor,
+                    TestExecutionResult.failed(ex));
+        } else if (event instanceof SuiteCompleted) {
             SuiteCompleted e = (SuiteCompleted) event;
             junitListener.executionFinished(
                     getOrCreateDescriptor(e.suiteId(), e.suiteName(), null),
                     TestExecutionResult.successful());
-        }
-
-        if (event instanceof RunStopped) {
+        } else if (event instanceof RunStopped) {
             junitListener.executionFinished(rootTestDescriptor, TestExecutionResult.aborted(null));
-        }
-        if (event instanceof RunCompleted) {
+        } else if (event instanceof RunCompleted) {
             junitListener.executionFinished(rootTestDescriptor, TestExecutionResult.successful());
+        } else if (event instanceof TestIgnored) {
+            TestIgnored e = (TestIgnored) event;
+            junitListener.executionSkipped(getOrCreateDescriptor(e.suiteId(), e.suiteName(), e.testName()), "ignored");
         }
     }
 
@@ -93,14 +89,6 @@ public void setSkipWithCause(Throwable skipWithCause) {
         this.skipWithCause = skipWithCause;
     }
 
-    private <T> T getOrElse(Option<T> option, T defaultValue) {
-        if (option.isDefined()) {
-            return option.get();
-        } else {
-            return defaultValue;
-        }
-    }
-
     private TestDescriptor getOrCreateDescriptor(String suiteId, String suiteName, String testName) {
         Optional<ScalatestDescriptor> existingDescriptor = ScalatestDescriptor.find(suiteId, testName);
         return existingDescriptor.orElseGet(() -> {

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

@@ -9,17 +9,18 @@
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestExecutionResult;
 import org.scalatest.*;
-import org.scalatest.events.Event;
-import org.scalatest.events.Ordinal;
-import org.scalatest.events.RunAborted$;
+import org.scalatest.events.*;
+import org.scalatest.exceptions.TestCanceledException;
+import org.scalatest.exceptions.TestFailedException;
+import org.scalatest.tools.SuiteResult$;
 import scala.Option;
-import scala.Option$;
 import scala.util.control.NonFatal$;
 
 import java.util.*;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import static co.helmethair.scalatest.scala.OptionHelper.getOrElse;
 import static co.helmethair.scalatest.scala.ScalaConversions.*;
 
 public class Executor {
@@ -125,26 +126,48 @@ private void runScalatests(ScalatestSuiteDescriptor containingSuite, List<Scalat
             status.waitUntilCompleted();
         } catch (Throwable e) {
             if (e instanceof InstantiationException || e instanceof IllegalAccessException) {
-                reporter.apply(runAborted(args.tracker().nextOrdinal(), e, Resources.cannotInstantiateSuite(e.getMessage())));
+                reporter.apply( suiteAborted(args.tracker().nextOrdinal(), e, Resources.cannotInstantiateSuite(e.getMessage()), scalasuite));
+            } else if (e instanceof TestFailedException ) {
+                reporter.apply( suiteAborted(args.tracker().nextOrdinal(),
+                        getOrElse(((TestFailedException) e).cause(),e), Resources.bigProblems(e), scalasuite));
             } else if (e instanceof NoClassDefFoundError) {
-                reporter.apply(runAborted(args.tracker().nextOrdinal(), e, Resources.cannotLoadClass(e.getMessage())));
+                reporter.apply(runAborted(args.tracker().nextOrdinal(), e, Resources.cannotLoadClass(e.getMessage()), scalasuite));
             } else {
-                reporter.apply(runAborted(args.tracker().nextOrdinal(), e, Resources.bigProblems(e)));
+                reporter.apply(runAborted(args.tracker().nextOrdinal(), e, Resources.bigProblems(e), scalasuite));
                 if (!NonFatal$.MODULE$.apply(e)) {
                     throw e;
                 }
             }
         }
     }
 
-    private Event runAborted(Ordinal ordinal, Throwable e, String reason) {
-        return RunAborted$.MODULE$.apply(ordinal,
+    private Event runAborted(Ordinal ordinal, Throwable e, String reason, Suite scalasuite) {
+        return RunAborted$.MODULE$.apply(
+                ordinal,
                 reason,
                 Option.apply(e),
                 Option.apply(null),
                 Option.apply(null),
                 Option.apply(null),
                 Option.apply(null),
+                Option.apply(scalasuite),
+                Thread.currentThread().getName(),
+                (new Date()).getTime()
+        );
+    }
+
+    private Event suiteAborted(Ordinal ordinal, Throwable e, String message, Suite suite) {
+        return SuiteAborted$.MODULE$.apply(
+                ordinal,
+                message,
+                suite.suiteName(),
+                suite.suiteId(),
+                Option.apply(suite.getClass().getName()),
+                Option.apply(e),
+                Option.apply(null),
+                Option.apply(null),
+                Option.apply(null),
+                Option.apply(null),
                 Option.apply(null),
                 Thread.currentThread().getName(),
                 (new Date()).getTime()

src/main/java/co/helmethair/scalatest/scala/OptionHelper.java

@@ -0,0 +1,14 @@
+package co.helmethair.scalatest.scala;
+
+import scala.Option;
+
+public class OptionHelper {
+    public static <T> T getOrElse(Option<T> option, T defaultValue) {
+        if (option.isDefined()) {
+            return option.get();
+        } else {
+            return defaultValue;
+        }
+    }
+
+}

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

@@ -46,4 +46,75 @@ void beforeAllAndAfterAllCalledTest() {
 
         verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
     }
+
+    @Test
+    void beforeFailedTest() {
+        EngineDiscoveryRequest discoveryRequest = createClassDiscoveryRequest("tests.FailInBeforeTest");
+        TestDescriptor discoveredTests = engine.discover(discoveryRequest, engineId);
+        TestEngineExecutionListener listener = spy(new TestEngineExecutionListener());
+        ExecutionRequest executionRequest = new ExecutionRequest(discoveredTests, listener, null);
+
+        Map<String, Integer> calls = new HashMap<String, Integer>() {{
+            put("before", 1);
+        }};
+
+        verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
+
+        verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInBeforeTest]", listener, null);
+    }
+
+    @Test
+    void beforeAllFailedTest() {
+        EngineDiscoveryRequest discoveryRequest = createClassDiscoveryRequest("tests.FailInBeforeAllTest");
+        TestDescriptor discoveredTests = engine.discover(discoveryRequest, engineId);
+        TestEngineExecutionListener listener = spy(new TestEngineExecutionListener());
+        ExecutionRequest executionRequest = new ExecutionRequest(discoveredTests, listener, null);
+
+        Map<String, Integer> calls = new HashMap<String, Integer>() {{
+            put("before", 1);
+            put("test 1 runs", 0);
+            put("test 2 runs", 0);
+        }};
+
+        verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
+        verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInBeforeAllTest]", listener, null);
+    }
+
+    @Test
+    void afterFailedTest() {
+        EngineDiscoveryRequest discoveryRequest = createClassDiscoveryRequest("tests.FailInAfterTest");
+        TestDescriptor discoveredTests = engine.discover(discoveryRequest, engineId);
+        TestEngineExecutionListener listener = spy(new TestEngineExecutionListener());
+        ExecutionRequest executionRequest = new ExecutionRequest(discoveredTests, listener, null);
+
+        Map<String, Integer> calls = new HashMap<String, Integer>() {{
+            put("after", 1);
+            put("runs", 1);
+        }};
+
+        verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
+
+        verifyTestSuccessReported("[engine:scalatest]/[suite:tests.FailInAfterTest]/[test:test]", listener);
+        verifyTestFailReportedWith("[engine:scalatest]/[suite:tests.FailInAfterTest]", listener, null);
+    }
+
+    @Test
+    void afterAllFailedTest() {
+        EngineDiscoveryRequest discoveryRequest = createClassDiscoveryRequest("tests.FailInAfterAllTest");
+        TestDescriptor discoveredTests = engine.discover(discoveryRequest, engineId);
+        TestEngineExecutionListener listener = spy(new TestEngineExecutionListener());
+        ExecutionRequest executionRequest = new ExecutionRequest(discoveredTests, listener, null);
+
+        Map<String, Integer> calls = new HashMap<String, Integer>() {{
+            put("after", 1);
+            put("test 1 runs", 1);
+            put("test 2 runs", 1);
+        }};
+
+        verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
+
+        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);
+    }
 }

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

@@ -0,0 +1,30 @@
+package co.helmethair.scalatest;
+
+import co.helmethair.scalatest.helper.TestEngineExecutionListener;
+import co.helmethair.scalatest.helper.TestHelpers;
+import org.junit.jupiter.api.Test;
+import org.junit.platform.engine.EngineDiscoveryRequest;
+import org.junit.platform.engine.ExecutionRequest;
+import org.junit.platform.engine.TestDescriptor;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.mockito.Mockito.spy;
+
+public class IgnoredTest implements TestHelpers {
+    @Test
+    void beforeAllAndAfterEachCalledTest() {
+        EngineDiscoveryRequest discoveryRequest = createClassDiscoveryRequest("tests.IgnoredTest");
+        TestDescriptor discoveredTests = engine.discover(discoveryRequest, engineId);
+        TestEngineExecutionListener listener = spy(new TestEngineExecutionListener());
+        ExecutionRequest executionRequest = new ExecutionRequest(discoveredTests, listener, null);
+
+        Map<String, Integer> calls = new HashMap<String, Integer>() {{
+            put("not ignored", 1);
+            put("ignored", 0);
+        }};
+
+        verifyTestExecuteCode(calls, () -> engine.execute(executionRequest));
+    }
+}

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

@@ -148,6 +148,16 @@ default void verifyTestFailReportedWith(String testIdsuffix, TestEngineExecution
         );
     }
 
+    default void verifyTestAbortedReportedWith(String testIdsuffix, TestEngineExecutionListener listener, Class<? extends Throwable> cause) {
+        verify(listener, atLeastOnce()).executionFinished(
+                argThat(a -> a.getUniqueId().toString().endsWith(testIdsuffix)),
+                argThat(a -> a.getThrowable().isPresent()
+                        && (cause == null || cause.isInstance(a.getThrowable().get()))
+                        && a.getStatus() == TestExecutionResult.Status.ABORTED
+                )
+        );
+    }
+
     default void verifyTestSkipReported(String testIdsuffix, TestEngineExecutionListener listener) {
         verify(listener, atLeastOnce()).executionFinished(
                 argThat(a -> a.getUniqueId().toString().endsWith(testIdsuffix)),

src/test/scala/tests/AssumeInBeforeTest.scala

@@ -0,0 +1,16 @@
+package tests
+
+import co.helmethair.scalatest.helper.RegisterCall
+import org.scalatest.BeforeAndAfter
+import org.scalatest.funsuite.AnyFunSuite
+
+class AssumeInBeforeTest extends AnyFunSuite with BeforeAndAfter with RegisterCall {
+
+  before {
+    assume(false, "this should be aborted")
+  }
+
+  test("some test") {
+
+  }
+}

src/test/scala/tests/FailInAfterAllTest.scala

@@ -0,0 +1,21 @@
+package tests
+
+import co.helmethair.scalatest.helper.RegisterCall
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.funsuite.AnyFunSuite
+
+class FailInAfterAllTest extends AnyFunSuite with BeforeAndAfterAll with RegisterCall {
+
+  override def afterAll() {
+    register("after")
+    fail()
+  }
+
+  test("test 1") {
+    register("test 1 runs")
+  }
+
+  test("test 2") {
+    register("test 2 runs")
+  }
+}

src/test/scala/tests/FailInAfterTest.scala

@@ -0,0 +1,22 @@
+package tests
+
+import co.helmethair.scalatest.helper.RegisterCall
+import org.scalatest.BeforeAndAfter
+import org.scalatest.funsuite.AnyFunSuite
+
+class FailInAfterTest extends AnyFunSuite with BeforeAndAfter with RegisterCall {
+
+  after {
+    register("after")
+    /*
+      As per Scalatest, only certain tests causing the test fail in an after block
+      "We will swallow an exception thrown from the after code if it is not test-aborting
+      and an exception was already thrown by beforeEach or test itself."
+     */
+    throw new ThreadDeath {}
+  }
+
+  test("test") {
+    register("runs")
+  }
+}