Running Scenario by line number from within Scenario doesn't work

[NAVA-PHOLLAND]

Summary

I used to be able to run cucumber feature_file_name.feature:XX with XX being a line anywhere in the scenario (usually the line that failed last time, to easily re-run a failure) but now it only works if XX is the number of the "Scenario" heading line.

Expected Behavior

cucumber feature_file_name.feature:XX should run the Scenario, from the beginning, that contains line XX.

Current Behavior

The Scenario does not run, and the following output is produced.

0 scenarios
0 steps
0m0.000s

Steps to Reproduce (for bugs)

1. Run any Scenario (with cucumber feature_file_name.feature:XX) from the command line, tagging the line number of any line within the scenario other than the "Scenario" heading

Your Environment

ruby 2.7.1
MacOS 10.15.6
cucumber-rails 2.1
cucumber 4.1.0

[aslakhellesoy]

Is this ruby, javascript or java? What cucumber version?

[NAVA-PHOLLAND]

Is this ruby, javascript or java? What cucumber version?
@aslakhellesoy

Updated to include environment

[aslakhellesoy]

Thanks for adding the cucumber-rails version. What cucumber version do you have?

[NAVA-PHOLLAND]

Thanks for adding the cucumber-rails version. What cucumber version do you have?

Updated again -- cucumber 4.1.0

[aslakhellesoy]

I'm unable to reproduce this with cucumber version 4.1.0. I tried with https://github.com/cucumber-ltd/shouty.rb (changed Gemspec to 4.1.0) and ran bundle exec cucumber features/hear_shout.feature:5 which runs a single scenario.

It also works fine with 5.1.0.

Is this a bug that only manifests itself with cucumber-rails? (sounds unlikely). Can you provide a Minimal, Reproducible Example?

[aslakhellesoy]

Oh, I see what you mean now - it no longer works if I specify line 6 instead of 5. Yep, that's definitely a regression. Never mind the Minimal, Reproducible Example.

[vincent-psarga]

Some explanations from @brasmusson: https://cucumberbdd.slack.com/archives/C62GZFLLT/p1595951692019300

The long story. Cucumber-Ruby was the first cucumber implementations. Originally the execution of feature file was done by iterating the AST. This made the cucumber implementation fragile and error prone. But the whole AST was available when executing, which means that the hooks could access the AST. In Cucumber-Ruby 2.0 the concept of compiling feature files was introduced. However the compiled test cases includes links back to the AST, so the whole AST was still available when executing, that is was still accessible from hooks. Then the concept of Pickles was created and the responsibility of compiling feature files became part of the responsibilities of the gherkin library (https://github.com/cucumber/cucumber/tree/master/gherkin#pickles).
The introduction of Pickles means IMHO that the central part of any cucumber implementation is a loop execute(Pickle), and it also specifies that the data available when executing is only the Pickle. Since only the Pickle is available when executing the hooks consequently can access only data of the Pickle - not the data of the AST from which the Pickle was compiled.
The gherkin compiler and Pickle was introduced into Cucumber-Ruby i v4, and consequently hooks can no longer access the AST. Since Cucumber-Ruby users have been used to be able to access the AST from hooks, many of them now miss being able to do that.
To me this issue boils down to this. a) Should Pickles be re-thought or not? Is executing feature files in cucumber implementations essentially executing Pickles compiled by the gherkin library? b) What is really the purpose of hooks? Is it to enable the execution, or is it to do all sort of stuff like reporting, logging etc. etc?
My best interpretation of the architecuture of Cucumber (TBH I have not that involved the last couple of years) is that a) executing Pickles is essentially what cucumber implementations do, and b) the purpose of hooks is to enable the execution (not reporting, logging etc - even though that is a very common misuse or hooks). Therefore my conclusion is the it should not be possible access the feature name in hooks. Which is not the popular conclusion I'm sure. (edited)

(I copy it here in case it disappears from Slack)

[thedeeno]

I'm seeing this behavior with cucumber (5.2.0) and cucumber-rails (2.2.0)

[luke-hill]

The cucumber version (v4 onwards is what triggered this). This appears to be all-encompassing around issue: #1432

[tiendo1011]

Does anyone know a workaround before this can be fixed?

[luke-hill]

The workaround @tiendo1011 is specify the line which explicitly calls Scenario. The "correct" behaviour. I use this term loosely is unaffected. It's more of the..... "You've given us something slightly incorrect, but we know you mean this" behaviour that has regressed.

As mentioned this is because the internals in cucumber4 have massively changed. Essentially the way in which cucumber works in v4 is completely new, even though to the user 99% will look identical. Ideally you should always be referring to Scenario line numbers, and not Given/When/Then line numbers, as those aren't representative of a Scenario/Test Case.

1. Feature: ABC
2. Scenario: XXX # <-- This is the only line number that will work in cucumber4 for now. So specify :2 in your runner
3. Given YYY
4. Then ZZZ

[botandrose]

Additionally, running :1 used to run all scenarios, now it runs zero.

[botandrose]

For context, my use-case is in the vim plugin I wrote: https://github.com/botandrose/vim-testkey. Basically I hit Enter in vim, and it runs whatever test is under my cursor. Super handy for tight TDD loop. This regression throws a wrench in that loop.

[botandrose]

I can imagine a hacky workaround in the CLI layer where we open the specified file, inspect the supplied line, and iterate upwards until we see Scenario:, and then mutate the supplied line number into that line number, removing it completely if Scenario: is never found, which would mean we're above the first scenario, and thus all scenarios should be run. I'll probably hack together something like this, unless there's a better idea.

[botandrose]

Got something working in the bin/cucumber binstub. Here be hackage:

#!/usr/bin/env ruby
require 'bundler/setup'

# hack in old fuzzy line number behavior from cucumber 3
require 'cucumber/cli/options'
Cucumber::Cli::Options.prepend Module.new {
  def parse!(args) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
    args.map! do |arg|
      line_numbers = arg.split(":")
      file = line_numbers.shift
      line_numbers.map! do |line_number|
        lines = File.readlines(file)
        line_number.to_i.downto(1).find do |i|
          lines[i-1] =~ /^\s+Scenario:/
        end
      end
      line_numbers.unshift(file)
      line_numbers.compact.join(":")
    end

    super
  end
}

load Gem.bin_path('cucumber', 'cucumber')

[botandrose]

I'd be happy to clean this up and create a PR to add it in as e.g. an off-by-default command line option, if there's a chance it could get merged!

[luke-hill]

I'm happy to review anything. The only caveat to be careful here, is we don't introduce "new" functionality. So if you wanted to revert to the old functionality, that is fine.

[botandrose]

@luke-hill Roger that. Totally makes sense to address this as a regression bugfix. Opening a PR with this in mind, presently.

[mattwynne]

I see @brasmusson's comments here: cucumber/cucumber-ruby-core@4747217

It seems like there's not quite enough info in the pickles for us to be able to match the lines within the scenario, or at least there wasn't when that commit was made. This surprises me @aslakhellesoy - do pickles not contain pickle-steps, and do pickle-steps not have a location?

[mattwynne]

Having had a closer look, it seems like we've added the Gherkin::Query since @brasmusson's commit, and that we're already using that utility's location method to look up the line of a step in a scenario. So @botandrose unless I'm missing something there's no major internal changes needed to get this working. 🎉

I suggest it's maybe just a matter of bringing back the old implementation of the Test::Case#match_locations? method:

https://github.com/cucumber/cucumber-ruby-core/blob/700ac0dd03070dff3b7949be698f938ce184b68c/lib/cucumber/core/test/case.rb#L78

[mattwynne]

I tracked down the tests for this; they were moved to the LocationsFilter's tests and I think they may have then got lost when the pickles compiler was implemented.

[botandrose]

Hi folks, I circled back around today to the newest 8.0 release to see if this had been fixed, but it appears the situation is worse now. I can't get the line number option to work at all... it always reports 0 scenarios.

[luke-hill]

@botandrose does this still work for the "correct" behaviour. i.e. running the line of the Scenario declaration? I believe we have a fair few unit tests for this.

[botandrose]

@luke-hill From my brief experimentation, no, there has been further regression of the line number functionality. Every line number I tried resulted in 0 scenarios, and I tried line numbers for tags, features, scenarios, and steps.

[mattwynne]

This is puzzling. We have a test case here which I would have expected to catch this kind of error.

Can you maybe try and reproduce it in a test, or at least a minimal reproducible example @botandrose?

[botandrose]

@luke-hill @mattwynne Ah, I found the culprit. The scenario was tagged with a tag that was disabled by cucumber.yml in the environment that I was in while testing. So my mistake, I have confirmed that there is no further regression, just the initial regressions recounted above:

1. Specifying a line within the body of a scenario used to match that scenario, now matches 0 scenarios
2. Specifying the line of the feature declaration, background declaration, or body of the background used to match all scenarios, now matches 0 scenarios.

I've willing to take a stab at fixing these two regressions, but I'll likely need some guidance! I've got both of them failing on a branch: https://github.com/botandrose/cucumber/pull/new/fix_cli_line_number_regressions . I'm going to dive in and try to reorient myself with the codebase... I haven't really looked since the 2.x days.

[botandrose]

@luke-hill @mattwynne Okay, I think I've tracked the issue it down to this method. https://github.com/cucumber/cucumber-ruby-core/blob/9b3c892c4056240be6542be05a2df6b5062b68e9/lib/cucumber/core/compiler.rb#L75

The private method #source_lines_for_pickle returns an array containing only the line number of the scenario declaration. I'm going to try to modify it to also contain every line number of the scenario body, the line number of the feature declaration, and the entire background declaration.

[botandrose]

Unsurprisingly, its not that simple. Too many other parts of cucumber are expecting the Test::Case's Location#lines to be only the Scenario line. I'm thinking the way forward is to add additional knowledge to Test::Case and Location::Precise, maybe call it matching_lines, and use that in Location::Precise#match?.

[botandrose]

Okay, put up a POC PR over at cucumber-core. What do y'all think?

[botandrose]

@mattwynne Ah, just now reviewing your comments from March 30th, 2021 about Test::Case#match_locations? and the fate of the relevant tests. I'll see if I can't resurrect those tests, and perhaps reuse the match_locations? method as well. It still exists, but appears to be currently unused by any of the gems in the cucumber family! Do you know of any current use of it?

[mattwynne]

@mattwynne Ah, just now reviewing your comments from March 30th, 2021 about Test::Case#match_locations? and the fate of the relevant tests. I'll see if I can't resurrect those tests, and perhaps reuse the match_locations? method as well. It still exists, but appears to be currently unused by any of the gems in the cucumber family! Do you know of any current use of it?

I'm afraid I don't have the code in my head well enough to know. I'd just search the code. There are some 3rd party libraries that use cucumber-ruby-core but we can always change the API or behaviour with a major release.

[botandrose]

@mattwynne @luke-hill Okay, there are two final pieces of missing functionality left to resolve this regression, and they are both related to running all the scenarios:

1. Specifying the line number of the Feature: line used to match all scenarios, now matches none.
2. Specifying the line number of the Background: line or its associated steps, used to match all scenarios, now matches none.

I saved these two items for last, because it looks like resolving them will require changes across multiple repos. Specifically, it seems to me that the Compiler needs to pass additional info regarding the feature and background down into the core TestCase object, so that it knows to match those lines. I've made some functional exploratory spikes around this strategy, but I'm not feeling super great about any them, to be honest. Since you two are much more familiar with the codebase, do you have any high-level implementation recommendations?

[mattwynne]

@botandrose do you want to book a pairing slot with me here? I reckon we could figure it out together.

[botandrose]

Yes, great idea! I'm in transit for the next day or two, but when I get settled in, I'll schedule a pair with you. Thank you!

…

On Thu, Dec 22, 2022, 23:58 Matt Wynne ***@***.***> wrote: @botandrose <https://github.com/botandrose> do you want to book a pairing slot with me here <https://calendly.com/mattwynne>? I reckon we could figure it out together. — Reply to this email directly, view it on GitHub <#1469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEH35TWB5NWEDBEGSZLT3WOU5P3ANCNFSM4QSJTCIQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[luke-hill]

@botandrose this will be one of the first items I tackle once I get v9 and v9.0.1 (or v9.1), released. I've currently got a plan for 4/6 PR's to be cut and released, then this is definitely next up

I remember reviewing a large portion of this and seeing that it's pretty close I don't envisage many blockers 🤞

[botandrose]

Hey Luke, that's great news! I would love to see this finally get merged in so I can use a mainline release after so many years of using my own fork. Thank you very much for your time and attention, and do let me know if there's anything I can do to help push this over the finish line.

…

On Tue, Aug 29, 2023 at 10:45 AM Luke Hill ***@***.***> wrote: @botandrose <https://github.com/botandrose> this will be one of the first items I tackle once I get v9 and v9.0.1 (or v9.1), released. I've currently got a plan for 4/6 PR's to be cut and released, then this is definitely next up I remember reviewing a large portion of this and seeing that it's pretty close I don't envisage many blockers 🤞 — Reply to this email directly, view it on GitHub <#1469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEH34AYZ7QUUORCAJ4Z7DXXWT3HANCNFSM4QSJTCIQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[luke-hill]

Hi @BARK-PHOLLAND

This in theory is now fully fixed and released in v9.2.0 on rubygems now. A massive shoutout needs to go to @botandrose for doing 95% of the work, with a few minor refactors and reviews from myself.

To anyone else monitoring this issue. Can you place a 👍 if this now is fixed for you and a 👎 if it's not fixed. Based on the emojis I see in the remainder of march I'll mark this as closed as fixed or not.

[luke-hill]

Closed as fixed and released in version 9.2.0 of cucumber-ruby