Fix Suite support (#87)

* Reproduce #54

* Dispatcher: Use NestedSuiteSelector / NestedTestSelector (SBT 1.3)

* Configuration: test identifier: filter out identifiers not containing source

- e.g. the platform (skipped by path.stream().skip(1)
- e.g. for JUnit 5 suites the engine used to start the test in the test suite

* TaskName: remove JUnit 5 suite name if present, add tests

* Adjust the expected value

---------

Co-authored-by: H. Habighorst <h.habighorst@protonmail.com>

src/library/src/main/java/com/github/sbt/junit/jupiter/internal/Configuration.java

@@ -23,11 +23,7 @@
 import com.github.sbt.junit.jupiter.internal.listeners.TreePrintingTestListener;
 import com.github.sbt.junit.jupiter.internal.options.Options;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Objects;
-import java.util.Optional;
+import java.util.*;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import org.junit.platform.engine.TestSource;
@@ -327,7 +323,9 @@ private List<TestIdentifier> getPath(TestPlan testPlan, TestIdentifier identifie
       List<TestIdentifier> result = new ArrayList<>();
 
       do {
-        result.add(identifier);
+        if (identifier.getSource().isPresent()) {
+          result.add(identifier);
+        }
         identifier = testPlan.getParent(identifier).orElse(null);
       } while (null != identifier);
 

src/library/src/main/java/com/github/sbt/junit/jupiter/internal/event/Dispatcher.java

@@ -27,14 +27,7 @@
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestExecutionResult;
 import org.junit.platform.launcher.TestIdentifier;
-import sbt.testing.Event;
-import sbt.testing.EventHandler;
-import sbt.testing.Fingerprint;
-import sbt.testing.OptionalThrowable;
-import sbt.testing.Selector;
-import sbt.testing.Status;
-import sbt.testing.SuiteSelector;
-import sbt.testing.TestSelector;
+import sbt.testing.*;
 
 /**
  * Dispatches test events to SBT.
@@ -170,17 +163,13 @@ static Selector toSelector(TaskName name) {
       if (null != name.nestedSuiteId()) {
         if (null != name.testName()) {
 
-          // FIXME: as soon as JUnitXmlTestsListener supports this
-          // return new NestedTestSelector(name.nestedSuiteId(), name.testName());
-          return new TestSelector(name.nestedSuiteId() + "#" + testName);
+          return new NestedTestSelector(name.nestedSuiteId(), name.testName());
         }
 
-        // FIXME: as soon as JUnitXmlTestsListener supports this
-        // return new NestedSuiteSelector(name.nestedSuiteId());
-        return new TestSelector(name.nestedSuiteId());
+        return new NestedSuiteSelector(name.nestedSuiteId());
       }
 
-      if (null != name.testName()) {
+      if (null != testName) {
 
         return new TestSelector(testName);
       }

src/library/src/main/java/com/github/sbt/junit/jupiter/internal/event/TaskName.java

@@ -85,6 +85,7 @@ static TaskName of(String testSuite, String testName) {
    * @return A task name representing the given identifier.
    */
   static TaskName of(String testSuite, TestIdentifier identifier) {
+    final String removedJunit5SuiteName = removeJunit5SuiteName(testSuite, identifier);
 
     TaskName result = new TaskName();
     result.fullyQualifiedName = testSuite;
@@ -94,7 +95,7 @@ static TaskName of(String testSuite, TestIdentifier identifier) {
     if (testSource instanceof ClassSource) {
 
       ClassSource classSource = (ClassSource) testSource;
-      result.nestedSuiteId = nestedSuiteId(testSuite, classSource.getClassName());
+      result.nestedSuiteId = nestedSuiteId(removedJunit5SuiteName, classSource.getClassName());
     }
 
     if (testSource instanceof MethodSource) {
@@ -109,6 +110,31 @@ static TaskName of(String testSuite, TestIdentifier identifier) {
     return result;
   }
 
+  /**
+   * Removes a leading JUnit 5 suite name from the test suite name, if present.
+   *
+   * @param testSuite The name of the enclosing test suite.
+   * @param identifier The test identifier.
+   * @return The suite identifier.
+   */
+  private static String removeJunit5SuiteName(String testSuite, TestIdentifier identifier) {
+    final List<UniqueId.Segment> suiteSegments =
+        identifier.getUniqueIdObject().getSegments().stream()
+            .filter(segment -> segment.getType().equals("suite"))
+            .collect(Collectors.toList());
+
+    if (suiteSegments.isEmpty()) {
+      return testSuite;
+    }
+
+    final String suiteIdentifier = suiteSegments.get(0).getValue();
+    if (!testSuite.startsWith(suiteIdentifier)) {
+      throw new RuntimeException(
+          "Test: " + testSuite + " does not start with JUnit 5 suite: " + suiteIdentifier);
+    }
+    return testSuite.substring(suiteIdentifier.length());
+  }
+
   /**
    * Extracts a nested test suite identifier if available.
    *

src/library/src/test/java/com/github/sbt/junit/jupiter/internal/event/DispatcherTest.java

@@ -37,7 +37,6 @@
 import junit.TestRunner;
 import org.hamcrest.FeatureMatcher;
 import org.hamcrest.Matcher;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import sbt.testing.Event;
@@ -135,7 +134,6 @@ public void shouldReportDynamicTests() {
   }
 
   @Test
-  @Ignore("Does not work with JUnitXmlTestsListener currently")
   public void shouldReportNestedTestsCorrectly() {
 
     testRunner.withArgs("-v");
@@ -152,26 +150,6 @@ public void shouldReportNestedTestsCorrectly() {
     assertThat(result, hasItem(selector(testName(equalTo(testName)))));
   }
 
-  /*
-   * Hack until NestedTestSelector is reported correctly by JUnitXmlTestsListener
-   */
-  @Test
-  public void shouldReportNestedTestsHackForJUnitXmlTestsListener() {
-
-    testRunner.withArgs("-v");
-    testRunner.execute(NestedTests.class);
-
-    List<Event> result = testRunner.eventHandler().byStatus(Status.Success);
-
-    String suiteName = ".event.DispatcherSampleTests$NestedTests";
-    String testName = "$First#testOfFirstNestedClass()";
-
-    assertThat(result, hasSize(1));
-    assertThat(result, hasItem(fullyQualifiedName(endsWith(suiteName))));
-    assertThat(result, hasItem(selector(instanceOf(TestSelector.class))));
-    assertThat(result, hasItem(selector(testName(equalTo(testName)))));
-  }
-
   private FeatureMatcher<Event, String> fullyQualifiedName(Matcher<String> matcher) {
     return new FeatureMatcher<Event, String>(matcher, "fullyQualifiedName", "fullyQualifiedName") {
       @Override

src/library/src/test/java/com/github/sbt/junit/jupiter/internal/listeners/FlatPrintingTestListenerFormatterTest.java

@@ -29,22 +29,18 @@ public static Object[][] samples() {
     // @formatter:off
 
     return new Object[][] {
-      {"jupiter.samples.NestedTests", "testOfFirstNestedClass", "{0}$First#{1}()"},
-      {"jupiter.samples.RepeatedTests", "repeatedTest", "{0}#{1}():#1"},
+      {"jupiter.samples.NestedTests", "testOfFirstNestedClass", "$First#{1}()"},
+      {"jupiter.samples.RepeatedTests", "repeatedTest", "#{1}():#1"},
       {
         "jupiter.samples.RepeatedTests",
         "repeatedTestWithRepetitionInfo",
-        "{0}#{1}(org.junit.jupiter.api.RepetitionInfo):#1"
+        "#{1}(org.junit.jupiter.api.RepetitionInfo):#1"
       },
-      {"jupiter.samples.SimpleTests", "firstTestMethod", "{0}#{1}()"},
-      {
-        "jupiter.samples.SimpleTests",
-        "testWithParameter",
-        "{0}#{1}(org.junit.jupiter.api.TestInfo)"
-      },
-      {"jupiter.samples.VintageTests", "vintageTestMethod", "{0}#{1}"},
-      {"jupiter.samples.VintageEnclosedTests", "testMethod", "{0}$NestedTest#{1}"},
-      {"jupiter.samples.VintageParameterizedTests", "testParameters", "{0}#{1}[A-65]"}
+      {"jupiter.samples.SimpleTests", "firstTestMethod", "#{1}()"},
+      {"jupiter.samples.SimpleTests", "testWithParameter", "#{1}(org.junit.jupiter.api.TestInfo)"},
+      {"jupiter.samples.VintageTests", "vintageTestMethod", "#{1}"},
+      {"jupiter.samples.VintageEnclosedTests", "testMethod", "$NestedTest#{1}"},
+      {"jupiter.samples.VintageParameterizedTests", "testParameters", "#{1}[A-65]"}
     };
 
     // @formatter:on

src/plugin/src/sbt-test/basic/suite-demo/build.sbt

@@ -0,0 +1,13 @@
+val junit             = Def.setting { "org.junit.jupiter"  % "junit-jupiter-api"           % JupiterKeys.junitJupiterVersion.value }
+val junitEngine       = Def.setting { "org.junit.jupiter"  % "junit-jupiter-engine"        % JupiterKeys.junitJupiterVersion.value }
+val junitRunner       = Def.setting { "org.junit.platform" % "junit-platform-runner"       % JupiterKeys.junitPlatformVersion.value }
+val junitSuiteEngine  = Def.setting { "org.junit.platform" % "junit-platform-suite-engine" % JupiterKeys.junitPlatformVersion.value }
+
+name := "test-project"
+libraryDependencies ++= Seq(
+  junit.value % Test,
+  junitEngine.value % Test,
+  junitRunner.value % Test,
+  junitSuiteEngine.value % Test,
+  "com.github.sbt.junit" % "jupiter-interface" % JupiterKeys.jupiterVersion.value % Test,
+)

src/plugin/src/sbt-test/basic/suite-demo/project/plugins.sbt

@@ -0,0 +1 @@
+addSbtPlugin("com.github.sbt.junit" % "sbt-jupiter-interface" % sys.props("project.version"))

src/plugin/src/sbt-test/basic/suite-demo/src/main/java/example/foo/A.java

@@ -0,0 +1,7 @@
+package example.foo;
+
+public class A {
+  public static String get() {
+    return "Test";
+  }
+}

src/plugin/src/sbt-test/basic/suite-demo/src/test/java/example/TestSuite.java

@@ -0,0 +1,12 @@
+package example;
+
+import org.junit.platform.suite.api.IncludeClassNamePatterns;
+import org.junit.platform.suite.api.SelectPackages;
+import org.junit.platform.suite.api.Suite;
+
+@Suite
+@SelectPackages("example.foo")
+@IncludeClassNamePatterns({".*Test"})
+public class TestSuite {
+
+}

src/plugin/src/sbt-test/basic/suite-demo/src/test/java/example/foo/ATest.java

@@ -0,0 +1,14 @@
+package example.foo;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+class ATest {
+
+  @Test
+  void testGet() {
+    assertEquals("Test", A.get());
+  }
+
+}

src/plugin/src/sbt-test/basic/suite-demo/test

@@ -0,0 +1 @@
+> test