updated base formatter to set a scenario as passed unless there exist…

roskee commented 6 months ago

🤔 What's changed?

I have updated the step evaluation in the base formatter to not set a scenario as passed just because some step has passed. A scenario, after this change, will only be passed only if no steps exist that are failed, pending or undefined in that scenario.

⚡️ What's your motivation?

Issue #581 was created because of this. I was able to reproduce it easily. All I had to do was update the [api test feature] like the following (only updated the method not allowed to method allowed

# file: version.feature
Feature: get version
  In order to know godog version
  As an API user
  I need to be able to request version

  Scenario: does not allow POST method
    When I send "POST" request to "/version"
    Then the response code should be 405
    And the response should match json:
      """
      {
        "error": "Method allowed"
      }
      """

  Scenario: should get version number
    When I send "GET" request to "/version"
    Then the response code should be 200
    And the response should match json:
      """
      {
        "version": "v0.0.0-dev"
      }
      """

The summary for the above change is as follows

As it can be seen from the picture, the total scenarios (2) does not much the sum of passed (2) and failed (1) scenarios. This is due to the steps overriding the scenario status. That is, if a failed step is evaluated first, it will set the scenario as failed. but the next passing step will override that state and make it as passed. But when the summary is prepared the number of failed scenarios is evaluated as the number of failed steps (since a failed step will always lead to a failed scenario and other dependent steps are skipped, this logic works). This leads to the incorrect sum in the above result.

🏷️ What kind of change is this?

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

♻️ Anything particular you want feedback on?

I am not sure if this issue is only on the base formatter implementation. If there are any other implementations of the formatter I should look into, please let me know :pray:

📋 Checklist:

[x] I agree to respect and uphold the Cucumber Community Code of Conduct
[x] 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.

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

github-actions[bot] commented 6 months ago

Go API Changes

# summary
Inferred base version: v0.13.0
Suggested version: v0.13.1

codecov[bot] commented 6 months ago

Codecov Report

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

Comparison is base (c61a939) 82.94% compared to head (542691e) 82.93%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #582 +/- ## ========================================== - Coverage 82.94% 82.93% -0.01% ========================================== Files 28 28 Lines 3412 3411 -1 ========================================== - Hits 2830 2829 -1 Misses 467 467 Partials 115 115 ``` | [Files](https://app.codecov.io/gh/cucumber/godog/pull/582?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber) | Coverage Δ | | |---|---|---| | [internal/formatters/fmt\_base.go](https://app.codecov.io/gh/cucumber/godog/pull/582?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber#diff-aW50ZXJuYWwvZm9ybWF0dGVycy9mbXRfYmFzZS5nbw==) | `87.20% <ø> (-0.08%)` | :arrow_down: |

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

roskee commented 6 months ago

@vearutop Is there anything I should update before you can approve this?

vearutop commented 6 months ago

@roskee thank you for fixing this!

cucumber / godog