From 21814ccfc9054a3610748d079b48f396095bbb1d Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Tue, 21 Mar 2023 23:50:57 +0100 Subject: [PATCH] [JUnit Platform Engine] Cache parsed features 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 --- CHANGELOG.md | 2 + .../platform/engine/CachingFeatureParser.java | 24 ++++++++++ .../engine/DiscoverySelectorResolver.java | 2 +- .../platform/engine/FeatureDescriptor.java | 4 ++ .../platform/engine/FeatureResolver.java | 2 +- .../junit/platform/engine/NodeDescriptor.java | 18 ++++--- .../engine/DiscoverySelectorResolverTest.java | 47 ++++++++++++++++++- 7 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/CachingFeatureParser.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac08ee839..76b2fa46d2 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/CachingFeatureParser.java b/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/CachingFeatureParser.java new file mode 100644 index 0000000000..621cda5fe3 --- /dev/null +++ b/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> cache = new HashMap<>(); + private final FeatureParser delegate; + + CachingFeatureParser(FeatureParser delegate) { + this.delegate = delegate; + } + + Optional parseResource(Resource resource) { + return cache.computeIfAbsent(resource.getUri(), uri -> delegate.parseResource(resource)); + } +} diff --git a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolver.java b/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolver.java index 9ecc186237..7ed92b20aa 100644 --- a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolver.java +++ b/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; diff --git a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureDescriptor.java b/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureDescriptor.java index 2e9b407e1f..b8d9d003ab 100644 --- a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureDescriptor.java +++ b/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureDescriptor.java @@ -20,6 +20,10 @@ class FeatureDescriptor extends AbstractTestDescriptor implements Node toKeep) { if (!toKeep.test(descriptor)) { if (descriptor.isTest()) { diff --git a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureResolver.java b/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureResolver.java index 991616c3ea..4ac0df8182 100644 --- a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/FeatureResolver.java +++ b/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 featureScanner = new ResourceScanner<>( ClassLoaders::getDefaultClassLoader, FeatureIdentifier::isFeature, diff --git a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/NodeDescriptor.java b/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/NodeDescriptor.java index 9d18e26ca0..5312941d2e 100644 --- a/cucumber-junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/NodeDescriptor.java +++ b/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 tags; private final Set 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 getTags(Pickle pickleEvent) { return pickleEvent.getTags().stream() .map(tag -> tag.substring(1)) @@ -136,7 +140,7 @@ public SkipResult shouldBeSkipped(CucumberEngineExecutionContext context) { private Optional 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 shouldBeSkippedByTagFilter(CucumberEngineExecutionC private Optional 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 shouldBeSkippedByNameFilter(CucumberEngineExecution public CucumberEngineExecutionContext execute( CucumberEngineExecutionContext context, DynamicTestExecutor dynamicTestExecutor ) { - context.runTestCase(pickleEvent); + context.runTestCase(pickle); return context; } diff --git a/cucumber-junit-platform-engine/src/test/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolverTest.java b/cucumber-junit-platform-engine/src/test/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolverTest.java index 07f91e0825..2a240907aa 100644 --- a/cucumber-junit-platform-engine/src/test/java/io/cucumber/junit/platform/engine/DiscoverySelectorResolverTest.java +++ b/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 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 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 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 selectSomePickle(DiscoverySelector resource) { + return selectAllPickles(resource).findFirst(); + } + + private Stream selectAllPickles(DiscoverySelector resource) { EngineDiscoveryRequest discoveryRequest = new SelectorRequest(resource); resolver.resolveSelectors(discoveryRequest, testDescriptor); Set descendants = testDescriptor.getDescendants(); resetTestDescriptor(); return descendants.stream() .filter(PickleDescriptor.class::isInstance) - .map(TestDescriptor::getUniqueId) - .findFirst(); + .map(TestDescriptor::getUniqueId); } @Test