[JUnit Platform Engine] Use FileSource.withPosition (#3084)

Creating a `FileSource` involves a call to `File.getCanonicalFile`.
This call can be slow because it involves the file system. Especially
if the file system is networked.

By using `FileSource.withPosition` we can reuse the canonical file from
the feature for its nodes.

Fixes: #3010

Author: mpkorstanje

.github/workflows/test-java.yml

@@ -17,6 +17,7 @@ on:
 jobs:
   build:
     strategy:
+      fail-fast: true
       matrix:
         os: [ ubuntu-latest, windows-latest ]
         version: [ 17, 21 ]

CHANGELOG.md

@@ -10,6 +10,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Fixed
+- [JUnit Platform Engine] Use `FileSource.withPosition` ([#3084](https://github.com/cucumber/cucumber-jvm/pull/3084) M.P. Korstanje)
+
 ### Changed
 - [JUnit Platform Engine] Use JUnit Platform 1.14.0 (JUnit Jupiter 5.14.0)
 

cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/CucumberDiscoverySelectors.java

@@ -1,7 +1,6 @@
 package io.cucumber.junit.platform.engine;
 
 import io.cucumber.core.feature.FeatureWithLines;
-import io.cucumber.core.gherkin.Feature;
 import io.cucumber.plugin.event.Node;
 import org.junit.platform.commons.util.ToStringBuilder;
 import org.junit.platform.engine.DiscoveryIssue;
@@ -44,7 +43,7 @@ static FeatureWithLinesSelector from(FeatureWithLines featureWithLines) {
         static Set<FeatureWithLinesSelector> from(UniqueId uniqueId) {
             return uniqueId.getSegments()
                     .stream()
-                    .filter(FeatureOrigin::isFeatureSegment)
+                    .filter(CucumberTestDescriptor::isFeatureSegment)
                     .map(featureSegment -> {
                         URI uri = URI.create(featureSegment.getValue());
                         Set<FilePosition> filePosition = getFilePosition(uniqueId.getLastSegment());
@@ -69,7 +68,7 @@ private static URI stripQuery(URI uri) {
         }
 
         private static Set<FilePosition> getFilePosition(UniqueId.Segment segment) {
-            if (FeatureOrigin.isFeatureSegment(segment)) {
+            if (CucumberTestDescriptor.isFeatureSegment(segment)) {
                 return Collections.emptySet();
             }
 
@@ -111,28 +110,28 @@ public String toString() {
 
     static class FeatureElementSelector implements DiscoverySelector {
 
-        private final Feature feature;
+        private final FeatureWithSource feature;
         private final Node element;
 
-        private FeatureElementSelector(Feature feature) {
-            this(feature, feature);
+        private FeatureElementSelector(FeatureWithSource feature) {
+            this(feature, feature.getFeature());
         }
 
-        private FeatureElementSelector(Feature feature, Node element) {
+        private FeatureElementSelector(FeatureWithSource feature, Node element) {
             this.feature = requireNonNull(feature);
             this.element = requireNonNull(element);
         }
 
-        static FeatureElementSelector selectFeature(Feature feature) {
+        static FeatureElementSelector selectFeature(FeatureWithSource feature) {
             return new FeatureElementSelector(feature);
         }
 
-        static FeatureElementSelector selectElement(Feature feature, Node element) {
+        static FeatureElementSelector selectElement(FeatureWithSource feature, Node element) {
             return new FeatureElementSelector(feature, element);
         }
 
         static Stream<FeatureElementSelector> selectElementsAt(
-                Feature feature, Supplier<Optional<Set<FilePosition>>> filePositions,
+                FeatureWithSource feature, Supplier<Optional<Set<FilePosition>>> filePositions,
                 DiscoveryIssueReporter issueReporter
         ) {
             return filePositions.get()
@@ -141,23 +140,25 @@ static Stream<FeatureElementSelector> selectElementsAt(
         }
 
         private static Stream<FeatureElementSelector> selectElementsAt(
-                Feature feature, Set<FilePosition> filePositions, DiscoveryIssueReporter issueReporter
+                FeatureWithSource feature, Set<FilePosition> filePositions, DiscoveryIssueReporter issueReporter
         ) {
             return filePositions.stream().map(filePosition -> selectElementAt(feature, filePosition, issueReporter));
         }
 
         static FeatureElementSelector selectElementAt(
-                Feature feature, Supplier<Optional<FilePosition>> filePosition, DiscoveryIssueReporter issueReporter
+                FeatureWithSource feature, Supplier<Optional<FilePosition>> filePosition,
+                DiscoveryIssueReporter issueReporter
         ) {
             return filePosition.get()
                     .map(position -> selectElementAt(feature, position, issueReporter))
                     .orElseGet(() -> selectFeature(feature));
         }
 
         static FeatureElementSelector selectElementAt(
-                Feature feature, FilePosition filePosition, DiscoveryIssueReporter issueReporter
+                FeatureWithSource feature, FilePosition filePosition, DiscoveryIssueReporter issueReporter
         ) {
-            return feature.findPathTo(candidate -> candidate.getLocation().getLine() == filePosition.getLine())
+            return feature.getFeature()
+                    .findPathTo(candidate -> candidate.getLocation().getLine() == filePosition.getLine())
                     .map(nodes -> nodes.get(nodes.size() - 1))
                     .map(node -> new FeatureElementSelector(feature, node))
                     .orElseGet(() -> {
@@ -167,7 +168,7 @@ static FeatureElementSelector selectElementAt(
         }
 
         private static void reportInvalidFilePosition(
-                Feature feature, FilePosition filePosition, DiscoveryIssueReporter issueReporter
+                FeatureWithSource feature, FilePosition filePosition, DiscoveryIssueReporter issueReporter
         ) {
             issueReporter.reportIssue(DiscoveryIssue.create(WARNING,
                 "Feature file " + feature.getUri()
@@ -176,7 +177,7 @@ private static void reportInvalidFilePosition(
                         ". Selecting the whole feature instead"));
         }
 
-        static Set<FeatureElementSelector> selectElementsOf(Feature feature, Node selected) {
+        static Set<FeatureElementSelector> selectElementsOf(FeatureWithSource feature, Node selected) {
             if (selected instanceof Node.Container) {
                 Node.Container<?> container = (Node.Container<?>) selected;
                 return container.elements().stream()
@@ -186,7 +187,7 @@ static Set<FeatureElementSelector> selectElementsOf(Feature feature, Node select
             return Collections.emptySet();
         }
 
-        Feature getFeature() {
+        FeatureWithSource getFeature() {
             return feature;
         }
 

cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/CucumberTestDescriptor.java

@@ -2,7 +2,11 @@
 
 import io.cucumber.core.gherkin.Feature;
 import io.cucumber.core.gherkin.Pickle;
+import io.cucumber.junit.platform.engine.CucumberTestDescriptor.FeatureElementDescriptor.ExamplesDescriptor;
+import io.cucumber.junit.platform.engine.CucumberTestDescriptor.FeatureElementDescriptor.RuleDescriptor;
+import io.cucumber.junit.platform.engine.CucumberTestDescriptor.FeatureElementDescriptor.ScenarioOutlineDescriptor;
 import io.cucumber.plugin.event.Location;
+import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestSource;
 import org.junit.platform.engine.TestTag;
 import org.junit.platform.engine.UniqueId;
@@ -17,16 +21,113 @@
 import java.util.Set;
 import java.util.stream.Stream;
 
+import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toSet;
 
 abstract class CucumberTestDescriptor extends AbstractTestDescriptor {
 
-    protected CucumberTestDescriptor(UniqueId uniqueId, String displayName, TestSource source) {
+    private static final String RULE_SEGMENT_TYPE = "rule";
+    private static final String FEATURE_SEGMENT_TYPE = "feature";
+    private static final String SCENARIO_SEGMENT_TYPE = "scenario";
+    private static final String EXAMPLES_SEGMENT_TYPE = "examples";
+    private static final String EXAMPLE_SEGMENT_TYPE = "example";
+
+    private CucumberTestDescriptor(UniqueId uniqueId, String displayName, TestSource source) {
         super(uniqueId, displayName, source);
     }
 
+    static Builder builder(CucumberConfiguration configuration) {
+        return new Builder(configuration, configuration.namingStrategy());
+    }
+
+    static class Builder {
+        private final CucumberConfiguration configuration;
+        private final NamingStrategy namingStrategy;
+
+        Builder(CucumberConfiguration configuration, NamingStrategy namingStrategy) {
+            this.configuration = requireNonNull(configuration);
+            this.namingStrategy = requireNonNull(namingStrategy);
+        }
+
+        Optional<TestDescriptor> build(
+                TestDescriptor parent, FeatureWithSource feature, io.cucumber.plugin.event.Node node
+        ) {
+            requireNonNull(parent);
+            requireNonNull(feature);
+            requireNonNull(node);
+
+            FeatureSource source = feature.getSource();
+            if (node instanceof io.cucumber.plugin.event.Node.Feature) {
+                return Optional.of(new FeatureDescriptor(
+                    createUniqueId(parent, FEATURE_SEGMENT_TYPE, feature.getUri()),
+                    namingStrategy.name(node),
+                    source.nodeSource(node),
+                    feature.getFeature()));
+            }
+
+            if (node instanceof io.cucumber.plugin.event.Node.Rule) {
+                return Optional.of(new RuleDescriptor(
+                    configuration,
+                    createUniqueId(parent, RULE_SEGMENT_TYPE, node.getLocation()),
+                    namingStrategy.name(node),
+                    source.nodeSource(node),
+                    node));
+            }
+
+            if (node instanceof io.cucumber.plugin.event.Node.Scenario) {
+                return Optional.of(new PickleDescriptor(
+                    configuration,
+                    createUniqueId(parent, SCENARIO_SEGMENT_TYPE, node.getLocation()),
+                    namingStrategy.name(node),
+                    source.nodeSource(node),
+                    feature.getFeature().getPickleAt(node)));
+            }
+
+            if (node instanceof io.cucumber.plugin.event.Node.ScenarioOutline) {
+                return Optional.of(new ScenarioOutlineDescriptor(
+                    configuration,
+                    createUniqueId(parent, SCENARIO_SEGMENT_TYPE, node.getLocation()),
+                    namingStrategy.name(node),
+                    source.nodeSource(node),
+                    node));
+            }
+
+            if (node instanceof io.cucumber.plugin.event.Node.Examples) {
+                return Optional.of(new ExamplesDescriptor(
+                    configuration,
+                    createUniqueId(parent, EXAMPLES_SEGMENT_TYPE, node.getLocation()),
+                    namingStrategy.name(node),
+                    source.nodeSource(node),
+                    node));
+            }
+
+            if (node instanceof io.cucumber.plugin.event.Node.Example) {
+                Pickle pickle = feature.getFeature().getPickleAt(node);
+                return Optional.of(new PickleDescriptor(
+                    configuration,
+                    createUniqueId(parent, EXAMPLE_SEGMENT_TYPE, node.getLocation()),
+                    namingStrategy.nameExample(node, pickle),
+                    source.nodeSource(node),
+                    pickle));
+            }
+            throw new IllegalStateException("Got a " + node.getClass() + " but didn't have a case to handle it");
+        }
+
+        private static UniqueId createUniqueId(TestDescriptor parent, String segmentType, Location line) {
+            return createUniqueId(parent, segmentType, String.valueOf(line.getLine()));
+        }
+
+        private static UniqueId createUniqueId(TestDescriptor parent, String segmentType, String line) {
+            return parent.getUniqueId().append(segmentType, line);
+        }
+    }
+
+    static boolean isFeatureSegment(UniqueId.Segment segment) {
+        return FEATURE_SEGMENT_TYPE.equals(segment.getType());
+    }
+
     protected abstract URI getUri();
 
     protected abstract Location getLocation();