Fixes #604 - remove line overwriting for scenario outlines in cucumber formatter

glibas commented 3 months ago

🤔 What's changed?

Removing line overwrite from cucumber formatter

if len(pickle.AstNodeIds) == 2 {
        _, row := feature.FindExample(pickle.AstNodeIds[1])
        line = row.Location.Line
    }

⚡️ What's your motivation?

The above code from cucumber/godog/fmt_cucumber.go overwrites the line property values of steps in cucumber reports (for Scenario Outlines only, regular Scenarios reports look good).

As a result, the step line property corresponds to the line in the Examples section rather than to the actual location of step within the feature file.

Due to the above, external tools that process the reports and use the line property for sorting the steps, display the steps out of order.

Other officially supported cucumber implementations (java, js, python, etc.) are generating reports correctly for Scenario Outline - line property corresponds to the actual location of step within the file rather than the line in Examples section.

🏷️ What kind of change is this?

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

♻️ Anything particular you want feedback on?

📋 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 updated tests to cover my changes.
[ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
[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.

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

github-actions[bot] commented 3 months ago

Go API Changes

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

codecov[bot] commented 3 months ago

Codecov Report

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

Comparison is base (5a5631a) 83.22% compared to head (4632c0a) 83.21%.

:exclamation: Current head 4632c0a differs from pull request most recent head 47145eb. Consider uploading reports for the commit 47145eb to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #605 +/- ## ========================================== - Coverage 83.22% 83.21% -0.02% ========================================== Files 28 28 Lines 3416 3413 -3 ========================================== - Hits 2843 2840 -3 Misses 458 458 Partials 115 115 ```

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

glibas commented 3 months ago

Hi @roskee @vearutop could you please take a look

roskee commented 3 months ago

@glibas could you perhaps share an example to reproduce the problem so that we can test it as well. And also link the original issue #604 so that it will be closed once this pull request is done. Thanks :slightly_smiling_face:

glibas commented 3 months ago

Hi @roskee , The issue is well illustrated in this test cucumber.json output scenario_outline. The line of the step is being overwritten with the line of the example in Examples table, so all the steps within one scenario outline instance have the same line number causing report generators which are using cucumber.json output to display steps in random order, since they rely on the line number.

glibas commented 3 months ago

Hi @roskee, also as an example can be used this scenario Scenario: Support of Feature Plus Scenario Outline With Steps which can be executed before the change and after the change to observe changes in lines numbering

glibas commented 3 months ago

Hi @roskee apologies for chasing on this, would you be able to have a look?

roskee commented 3 months ago

Sorry for being late. I was very busy. I will take a look and let you know my thoughts.

glibas commented 3 months ago

No problem, thank you. In essence the change aligns godog cucumber json reporting behavior for scenario outlines with other cucumber implementations so hopefully should be straight forward.

glibas commented 3 months ago

Thank you @roskee for confirming!

cucumber / godog