[JUnit Platform Engine] Cache parsed features (#2711)

The JUnit Platform allows Scenarios and Examples to be selected by their
unique id. This id is stable between test executions and can be used to rerun
failing Scenarios and Examples.

For practical reasons each unique id is processed individually[+] and results
in a feature file being parsed. The parsed feature is then compiled into
pickles. Both are mapped to a hierarchy of JUnit 5 test descriptors. These are
then merged. The merge process will discard any duplicate nodes.

Each time a feature file is parsed the parser will assign unique identifiers
to all ast nodes and pickles. As a result the merged hierarchy of junit test
descriptors contained pickles that belonged to a different feature file.

This poses a problem for tools such as the junit-xml-formatter that depend on
the identifiers in pickles and features to stitch everything back together. By
caching the parsed feature files we ensure that within a single execution the
internal identifiers do not change.

 + : It would actually matter little if we did process all unique ids at once,
     the problem would persist when uri selectors are used, either alone or in
     combination with unique id selectors.

Fixes: #2709

Author: mpkorstanje

CHANGELOG.md

@@ -10,6 +10,8 @@ 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] Corrupted junit-xml report when using `surefire.rerunFailingTestsCount` parameter ([#2709](https://github.com/cucumber/cucumber-jvm/pull/2709) M.P. Korstanje)
 
 ## [7.11.1] - 2023-01-27
 ### Added

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

@@ -0,0 +1,24 @@
+package io.cucumber.junit.platform.engine;
+
+import io.cucumber.core.feature.FeatureParser;
+import io.cucumber.core.gherkin.Feature;
+import io.cucumber.core.resource.Resource;
+
+import java.net.URI;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+class CachingFeatureParser {
+
+    private final Map<URI, Optional<Feature>> cache = new HashMap<>();
+    private final FeatureParser delegate;
+
+    CachingFeatureParser(FeatureParser delegate) {
+        this.delegate = delegate;
+    }
+
+    Optional<Feature> parseResource(Resource resource) {
+        return cache.computeIfAbsent(resource.getUri(), uri -> delegate.parseResource(resource));
+    }
+}

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

@@ -27,7 +27,7 @@
 
 class DiscoverySelectorResolver {
 
-    private static final Logger log = LoggerFactory.getLogger(FeatureResolver.class);
+    private static final Logger log = LoggerFactory.getLogger(DiscoverySelectorResolver.class);
 
     private static boolean warnedWhenCucumberFeaturesPropertyIsUsed = false;
 

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

@@ -20,6 +20,10 @@ class FeatureDescriptor extends AbstractTestDescriptor implements Node<CucumberE
         this.feature = feature;
     }
 
+    Feature getFeature() {
+        return feature;
+    }
+
     private static void pruneRecursively(TestDescriptor descriptor, Predicate<TestDescriptor> toKeep) {
         if (!toKeep.test(descriptor)) {
             if (descriptor.isTest()) {

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

@@ -38,7 +38,7 @@ final class FeatureResolver {
 
     private static final Logger log = LoggerFactory.getLogger(FeatureResolver.class);
 
-    private final FeatureParser featureParser = new FeatureParser(UUID::randomUUID);
+    private final CachingFeatureParser featureParser = new CachingFeatureParser(new FeatureParser(UUID::randomUUID));
     private final ResourceScanner<Feature> featureScanner = new ResourceScanner<>(
         ClassLoaders::getDefaultClassLoader,
         FeatureIdentifier::isFeature,

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

@@ -89,17 +89,17 @@ public Type getType() {
 
     static final class PickleDescriptor extends NodeDescriptor {
 
-        private final Pickle pickleEvent;
+        private final Pickle pickle;
         private final Set<TestTag> tags;
         private final Set<ExclusiveResource> exclusiveResources = new LinkedHashSet<>(0);
 
         PickleDescriptor(
                 ConfigurationParameters parameters, UniqueId uniqueId, String name, TestSource source,
-                Pickle pickleEvent
+                Pickle pickle
         ) {
             super(parameters, uniqueId, name, source);
-            this.pickleEvent = pickleEvent;
-            this.tags = getTags(pickleEvent);
+            this.pickle = pickle;
+            this.tags = getTags(pickle);
             this.tags.forEach(tag -> {
                 ExclusiveResourceOptions exclusiveResourceOptions = new ExclusiveResourceOptions(parameters, tag);
                 exclusiveResourceOptions.exclusiveReadWriteResource()
@@ -111,6 +111,10 @@ static final class PickleDescriptor extends NodeDescriptor {
             });
         }
 
+        Pickle getPickle() {
+            return pickle;
+        }
+
         private Set<TestTag> getTags(Pickle pickleEvent) {
             return pickleEvent.getTags().stream()
                     .map(tag -> tag.substring(1))
@@ -136,7 +140,7 @@ public SkipResult shouldBeSkipped(CucumberEngineExecutionContext context) {
 
         private Optional<SkipResult> shouldBeSkippedByTagFilter(CucumberEngineExecutionContext context) {
             return context.getOptions().tagFilter().map(expression -> {
-                if (expression.evaluate(pickleEvent.getTags())) {
+                if (expression.evaluate(pickle.getTags())) {
                     return SkipResult.doNotSkip();
                 }
                 return SkipResult
@@ -148,7 +152,7 @@ private Optional<SkipResult> shouldBeSkippedByTagFilter(CucumberEngineExecutionC
 
         private Optional<SkipResult> shouldBeSkippedByNameFilter(CucumberEngineExecutionContext context) {
             return context.getOptions().nameFilter().map(pattern -> {
-                if (pattern.matcher(pickleEvent.getName()).matches()) {
+                if (pattern.matcher(pickle.getName()).matches()) {
                     return SkipResult.doNotSkip();
                 }
                 return SkipResult
@@ -161,7 +165,7 @@ private Optional<SkipResult> shouldBeSkippedByNameFilter(CucumberEngineExecution
         public CucumberEngineExecutionContext execute(
                 CucumberEngineExecutionContext context, DynamicTestExecutor dynamicTestExecutor
         ) {
-            context.runTestCase(pickleEvent);
+            context.runTestCase(pickle);
             return context;
         }
 

cucumber-junit-platform-engine/src/test/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolverTest.java

@@ -1,5 +1,7 @@
 package io.cucumber.junit.platform.engine;
 
+import io.cucumber.core.gherkin.Feature;
+import io.cucumber.core.gherkin.Pickle;
 import io.cucumber.core.logging.LogRecordListener;
 import io.cucumber.junit.platform.engine.NodeDescriptor.PickleDescriptor;
 import io.cucumber.junit.platform.engine.nofeatures.NoFeatures;
@@ -23,6 +25,7 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -33,6 +36,7 @@
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static io.cucumber.junit.platform.engine.Constants.FEATURES_PROPERTY_NAME;
 import static java.util.Collections.singleton;
@@ -382,15 +386,54 @@ void resolveRequestWithMultipleUniqueIdSelector() {
                     .collect(toSet()));
     }
 
+    @Test
+    void resolveRequestWithMultipleUniqueIdSelectorFromTheSameFeature() {
+        Set<UniqueId> selectors = new HashSet<>();
+
+        DiscoverySelector resource = selectDirectory(
+            "src/test/resources/io/cucumber/junit/platform/engine/feature-with-outline.feature");
+        selectAllPickles(resource).forEach(selectors::add);
+
+        EngineDiscoveryRequest discoveryRequest = new SelectorRequest(
+            selectors.stream()
+                    .map(DiscoverySelectors::selectUniqueId)
+                    .collect(Collectors.toList()));
+
+        resolver.resolveSelectors(discoveryRequest, testDescriptor);
+
+        Set<String> pickleIdsFromFeature = testDescriptor.getDescendants()
+                .stream()
+                .filter(FeatureDescriptor.class::isInstance)
+                .map(FeatureDescriptor.class::cast)
+                .map(FeatureDescriptor::getFeature)
+                .map(Feature::getPickles)
+                .flatMap(Collection::stream)
+                .map(Pickle::getId)
+                .collect(toSet());
+
+        Set<String> pickleIdsFromPickles = testDescriptor.getDescendants()
+                .stream()
+                .filter(PickleDescriptor.class::isInstance)
+                .map(PickleDescriptor.class::cast)
+                .map(PickleDescriptor::getPickle)
+                .map(Pickle::getId)
+                .collect(toSet());
+
+        assertEquals(pickleIdsFromFeature, pickleIdsFromPickles);
+    }
+
     private Optional<UniqueId> selectSomePickle(DiscoverySelector resource) {
+        return selectAllPickles(resource).findFirst();
+    }
+
+    private Stream<UniqueId> selectAllPickles(DiscoverySelector resource) {
         EngineDiscoveryRequest discoveryRequest = new SelectorRequest(resource);
         resolver.resolveSelectors(discoveryRequest, testDescriptor);
         Set<? extends TestDescriptor> descendants = testDescriptor.getDescendants();
         resetTestDescriptor();
         return descendants.stream()
                 .filter(PickleDescriptor.class::isInstance)
-                .map(TestDescriptor::getUniqueId)
-                .findFirst();
+                .map(TestDescriptor::getUniqueId);
     }
 
     @Test