godog icon indicating copy to clipboard operation
godog copied to clipboard

fix(formatter): On concurrent execution, execute formatter at end of Scenario

Open tigh-latte opened this issue 1 year ago • 2 comments

🤔 What's changed?

Add a new formatter type which only executes when Flush is called. Use this formatter when running concurrently.

Then while running concurrently, after the suite is copied for a scenario, we overwrite the copy's Formatter to use this new OnFlushWrap formatter.

At the end of runPickle, if we can Flush, we do.

⚡️ What's your motivation?

Fixes #643 , Scenario output is only printed at end of runPickle func when running concurrently.

🏷️ What kind of change is this?

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

♻️ Anything particular you want feedback on?

Any feedback on the general approach to solving this would be good.

📋 Checklist:

  • [x] I agree to respect and uphold the Cucumber Community Code of Conduct
  • [x] I've changed the behaviour of the code
    • [x] 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
    • [x] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

tigh-latte avatar Oct 05 '24 13:10 tigh-latte

one test is broken, Test_FormatterConcurrencyRun but on when running scenario_outline.feature, keep this as a draft until I work out why.

tigh-latte avatar Oct 05 '24 15:10 tigh-latte

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.79%. Comparing base (153db4e) to head (b3b7d25). Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #645      +/-   ##
==========================================
- Coverage   83.21%   79.79%   -3.42%     
==========================================
  Files          28       41      +13     
  Lines        3413     4044     +631     
==========================================
+ Hits         2840     3227     +387     
- Misses        458      697     +239     
- Partials      115      120       +5     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 05 '24 19:10 codecov[bot]

Thank you!

vearutop avatar Nov 08 '24 16:11 vearutop

if this is a new formatter please, can we please have fmt-output-test cases in line with the other formatters please .

consistent test patterns help other commiters helps avoid entropy

Johnlon avatar Nov 08 '24 22:11 Johnlon

@Johnlon it isn't really a new formatter and instead is a wrapper of current formatters. All it does is delay the execution of the wrapped formatter until the end of the scenario, and this delayed execution is tested.

In an additional PR, I could put time into testing that the output of a wrapped formatter is as we would expect, but given the wrapper only provides behavioural changes, testing the behaviour as I've done here and using the output tests already in place is enough.

tigh-latte avatar Nov 09 '24 00:11 tigh-latte

@tigh-latte Thanks so much for adding this, I've been able to re-enable concurrency in my tests because of it!

thomshutt avatar Nov 14 '24 12:11 thomshutt