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

BARK-PHOLLAND commented 4 years ago

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)

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 commented 4 years ago

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

BARK-PHOLLAND commented 4 years ago

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

Updated to include environment

aslakhellesoy commented 4 years ago

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

BARK-PHOLLAND commented 4 years ago

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

Updated again -- cucumber 4.1.0

aslakhellesoy commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

luke-hill commented 4 years ago

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

tiendo1011 commented 3 years ago

Does anyone know a workaround before this can be fixed?

luke-hill commented 3 years ago

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.

Feature: ABC

Scenario: XXX # <-- This is the only line number that will work in cucumber4 for now. So specify :2 in your runner

Given YYY

Then ZZZ

botandrose commented 3 years ago

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

botandrose commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

mattwynne commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.