Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Oct 27, 2023

Conversation

roskee
Copy link
Member

@roskee roskee commented Oct 23, 2023

🤔 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
image

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 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 🙏

📋 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.

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

@roskee roskee linked an issue Oct 23, 2023 that may be closed by this pull request
@github-actions
Copy link

Go API Changes

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

@vearutop vearutop force-pushed the 581-incorrect-count-of-passed-scenarios branch from 2f6b096 to 542691e Compare October 23, 2023 22:11
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            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 Coverage Δ
internal/formatters/fmt_base.go 87.20% <ø> (-0.08%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@roskee roskee requested a review from vearutop October 24, 2023 04:29
@roskee
Copy link
Member Author

roskee commented Oct 24, 2023

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

@vearutop
Copy link
Member

@roskee thank you for fixing this!

@vearutop vearutop merged commit 25274b0 into main Oct 27, 2023
8 checks passed
@vearutop vearutop deleted the 581-incorrect-count-of-passed-scenarios branch October 27, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why Getting total scenarios as 6 while I am having total 5 scenarios
2 participants