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

CLI: Targetting the same file multiple times seems to duplicate the results #1706

Closed
nicojs opened this issue Jun 25, 2021 · 6 comments · Fixed by #1708
Closed

CLI: Targetting the same file multiple times seems to duplicate the results #1706

nicojs opened this issue Jun 25, 2021 · 6 comments · Fixed by #1708
Labels
🐛 bug Defect / Bug good first issue Good for newcomers ✅ accepted The core team has agreed that it is a good idea to fix this

Comments

@nicojs
Copy link
Contributor

nicojs commented Jun 25, 2021

Describe the bug

In order to save time, I'm trying to use cucumber-js to target individual features and examples (a lot of them). I'm using these arguments:

$ npx cucumber-js "features/foo.feature:33" "features/foo.feature:34"

(to target 2 examples)

However, instead of saving time, it just takes more time. It seems that both examples are now executed twice.

To Reproduce

  1. Follow the nodejs example: https://github.com/cucumber/cucumber-js/blob/main/docs/nodejs_example.md
  2. Execute npx cucumber-js features/simple_math.feature:19, see:
    ...
    
    1 scenario (1 passed)
    3 steps (3 passed)
    
  3. Same for line 20, npx cucumber-js features/simple_math.feature:20, see:
    ...
    
    1 scenario (1 passed)
    3 steps (3 passed)
    
  4. Now, execute npx cucumber-js features/simple_math.feature:19 features/simple_math.feature:20, see:
    ............
    
    4 scenarios (4 passed)
    12 steps (12 passed)
    

Expected behavior

...
2 scenario (2 passed)
6 steps (6 passed)

Desktop (please complete the following information):

  • OS: Windows
  • cucumberjs v7.3.0
  • npm 6.14.11, node v12.21.0

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]
@davidjgoss
Copy link
Contributor

Thanks for the report @nicojs.

Just to clarify, it seems like you're running into this when using a Scenario Outline with an Examples table. I assume it doesn't occur with regular Scenarios?

@nicojs
Copy link
Contributor Author

nicojs commented Jun 25, 2021

It also occurs with regular scenarios.

I've dug a little deeper and found out the issue:

async expandFeaturePaths(featurePaths: string[]): Promise<string[]> {
featurePaths = featurePaths.map((p) => p.replace(/(:\d+)*$/g, '')) // Strip line numbers
return this.expandPaths(featurePaths, '.feature')
}

Feature files are expanded but not deduplicated. This fixes it:

 async expandFeaturePaths(featurePaths: string[]): Promise<string[]> { 
   featurePaths = featurePaths.map((p) => p.replace(/(:\d+)*$/g, '')) // Strip line numbers 
+  featurePaths = [...new Set(featurePaths)]; // Deduplicate the feature files
   return this.expandPaths(featurePaths, '.feature') 
 } 

You want me to create a PR? Does this warrant a feature file? Or unit testing?

@nicojs
Copy link
Contributor Author

nicojs commented Jun 25, 2021

The fix seems straight forward enough to not deem itself worthy for a feature file IMHO 🤷‍♂️ See PR #1708

@davidjgoss
Copy link
Contributor

Ah thanks, good find.

FWIW we do have a notation that supports targeting multiple lines in the same file, check out this scenario https://github.com/cucumber/cucumber-js/blob/main/features/target_specific_scenarios_by_line.feature#L40.

If you're willing to do a PR to add the deduplication though that would be great. I think another scenario in the aforementioned file (as I think that's the most likely circumstance to hit this) plus relevant unit testing would be right in terms of coverage.

@davidjgoss davidjgoss added 🐛 bug Defect / Bug good first issue Good for newcomers ✅ accepted The core team has agreed that it is a good idea to fix this labels Jun 25, 2021
@nicojs
Copy link
Contributor Author

nicojs commented Jun 25, 2021

Ah cool! I was looking for exactly that 😆I will use that then

If you're willing to do a PR to add the deduplication though that would be great. I think another scenario in the aforementioned file (as I think that's the most likely circumstance to hit this) plus relevant unit testing would be right in terms of coverage.

PR is ready. I will add the scenario too, no problem.

@nicojs
Copy link
Contributor Author

nicojs commented Jun 25, 2021

Ok, added it. I've transformed the scenario to a scenario outline with 2 examples. I've also manually "mutation tested" it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Defect / Bug good first issue Good for newcomers ✅ accepted The core team has agreed that it is a good idea to fix this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants