[JUnit Platform Engine] Cache parsed features

[mpkorstanje]

🤔 What's changed?

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

🏷️ What kind of change is this?

• 🐛 Bug fix (non-breaking change which fixes a defect)

📋 Checklist:

• I agree to respect and uphold the Cucumber Community Code of Conduct
• I've changed the behaviour of the code
  • I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

[jkronegg]

I checked on my project (with CLI Main or JUnit5 platform, but without rerun of failed tests) : the feature files are parsed only once. So this caching will make no performance difference.

I which setup do you expect to have better performance ?

[mpkorstanje]

Yeah sorry, this is just a draft. I didn't get around to a full write up of the problem.

I would expect this to have a benefit when tests are selected by their uniqueId. Currently for every uniqueId we reparse the feature file.

But the performance improvement is incidental to fixing the underlying problems caused by parsing multiple times and discarding some of the data.

[jkronegg]

You mean that each failed test makes all features to be parsed again ? In this case, yes, caching will help.

[codecov]

Codecov Report

Merging #2711 (21814cc) into main (12c5029) will increase coverage by 0.01%.
The diff coverage is 92.85%.

@@             Coverage Diff              @@
##               main    #2711      +/-   ##
============================================
+ Coverage     84.68%   84.69%   +0.01%     
- Complexity     2682     2685       +3     
============================================
  Files           322      323       +1     
  Lines          9449     9456       +7     
  Branches        899      899              
============================================
+ Hits           8002     8009       +7     
  Misses         1120     1120              
  Partials        327      327              

Impacted Files	Coverage Δ
...cucumber/junit/platform/engine/NodeDescriptor.java	97.18% <83.33%> (+0.04%)	⬆️
...er/junit/platform/engine/CachingFeatureParser.java	100.00% <100.00%> (ø)
...nit/platform/engine/DiscoverySelectorResolver.java	95.74% <100.00%> (ø)
...umber/junit/platform/engine/FeatureDescriptor.java	100.00% <100.00%> (ø)
...ucumber/junit/platform/engine/FeatureResolver.java	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.