cucumber-ruby icon indicating copy to clipboard operation
cucumber-ruby copied to clipboard

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

Open BARK-PHOLLAND opened this issue 3 years ago • 31 comments

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

BARK-PHOLLAND avatar Sep 01 '20 16:09 BARK-PHOLLAND

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

aslakhellesoy avatar Sep 01 '20 18:09 aslakhellesoy

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

Updated to include environment

BARK-PHOLLAND avatar Sep 01 '20 18:09 BARK-PHOLLAND

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

aslakhellesoy avatar Sep 01 '20 18:09 aslakhellesoy

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

Updated again -- cucumber 4.1.0

BARK-PHOLLAND avatar Sep 01 '20 18:09 BARK-PHOLLAND

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 avatar Sep 01 '20 22:09 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.

aslakhellesoy avatar Sep 01 '20 22:09 aslakhellesoy

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)

vincent-psarga avatar Sep 04 '20 14:09 vincent-psarga

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

thedeeno avatar Nov 23 '20 19:11 thedeeno

The cucumber version (v4 onwards is what triggered this). This appears to be all-encompassing around issue: https://github.com/cucumber/cucumber-ruby/issues/1432

luke-hill avatar Nov 24 '20 09:11 luke-hill

Does anyone know a workaround before this can be fixed?

tiendo1011 avatar Jan 03 '21 03:01 tiendo1011

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

luke-hill avatar Jan 05 '21 09:01 luke-hill

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

botandrose avatar Jan 12 '21 23:01 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 avatar Jan 12 '21 23:01 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 avatar Jan 12 '21 23:01 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 avatar Jan 13 '21 00:01 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!

botandrose avatar Jan 13 '21 00:01 botandrose

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.

luke-hill avatar Jan 13 '21 10:01 luke-hill

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

botandrose avatar Jan 13 '21 16:01 botandrose

I see @brasmusson's comments here: https://github.com/cucumber/cucumber-ruby-core/commit/47472171c8ff183f06d0e86606663b27f3210674

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 avatar Mar 30 '21 05:03 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 avatar Mar 30 '21 05:03 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.

mattwynne avatar Mar 30 '21 06:03 mattwynne

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.

botandrose avatar Aug 11 '22 11:08 botandrose

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

luke-hill avatar Aug 11 '22 11:08 luke-hill

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

botandrose avatar Aug 11 '22 11:08 botandrose

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?

mattwynne avatar Aug 12 '22 18:08 mattwynne

@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 avatar Aug 14 '22 11:08 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 avatar Aug 14 '22 11:08 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 avatar Aug 14 '22 12:08 botandrose

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

botandrose avatar Aug 14 '22 13:08 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?

botandrose avatar Aug 14 '22 13:08 botandrose