fix logFailedTestCases for RerunFormatter

furiel commented 1 year ago

Referring to https://github.com/cucumber/cucumber-js/issues/2290

What's changed?

RerunFormatter used to log failed attempts even if the attempt was retried. For example, it was possible that a testcase might have been fixed with one of the retry attempts, but still was logged due to earlier failing attempt. From now, RerunFormatter does not log attempts that are retried.
Some minor quality of life changes for users who would like to extends RerunFormatter.

What's your motivation?

I am using retry to stabilize tests of a distributed system. It was possible (for example right after a deployment), that the first couple of test attempts are failed against a service. I am also using RerunFormatter to log the failures. However, as RerunFormatter logged the failed attempts too, it was difficult to find out what testcases I needed to look at in case of a regression.

I attempted to extend RerunFormatter according to my needs, which turned out to be more complex than I anticipated.

I created an issue and this patchet about both behavior of the RerunFormatter, and the challenges of it's customization.

What kind of change is this?

:bug: RerunFormatter's logFailedTestCases output is changed.
:bank: Minor refactors and changes to RerunFormatter to simplify extends-ing from it.

Anything particular you want feedback on?

This is my first time I am working with typescript/javascript. I appreciate any feedbacks, let it be small or complex. However, I may need a bit more guidance due to my infamiliarity with the language. Do not hesitate to overexplain the change requests. :) I also set Allow edits by maintainers if you feel something is easier for you to change, instead of guiding me through.

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.
[x] Users should know about my change
- [x] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

coveralls commented 1 year ago

coverage: 98.499% (+0.002%) from 98.497% when pulling d6db66db53f43145aea045abbbd4123c237d08e6 on furiel:unstable-tests into 8fece8387546610ae578e49ae5675620548fcecc on cucumber:main.

aslakhellesoy commented 1 year ago

Hi @furiel,

Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾

In return for this generous offer we hope you will:

✅ Continue to use branches and pull requests. When someone on the core team approves a pull request (yours or someone else's), you're welcome to merge it yourself.
💚 Commit to setting a good example by following and upholding our code of conduct in your interactions with other collaborators and users.
💬 Join the community Slack channel to meet the rest of the team and make yourself at home.
ℹ️ Don't feel obliged to help, just do what you can if you have the time and the energy.
🙋 Ask if you need anything. We're looking for feedback about how to make the project more welcoming, so please tell us!

On behalf of the Cucumber core team, Aslak Hellesøy Creator of Cucumber

cucumber / cucumber-js