The SPEC environment variable does not support using bracketed example IDs

[ezekg]

Your environment

• Ruby version: 3.2.2
• rspec-core version: 3.12.0

Steps to reproduce

Using the SPEC environment variable with brackets does not work with the spec Rake task.

SPEC=spec/example_spec.rb[1:2] rake spec

Expected behavior

Only the provided example IDs are run.

Actual behavior

All examples are run, as if SPEC was not provided.

Cause

The problem is that Rake::FileList recognizes the opening bracket as a glob pattern and passes it to Dir.glob which fails to find the filename because it doesn't support globbing on line number, etc. (as expected).

Rake::FileList['spec/example_spec.rb[1:2:3:4]']
# => []
Dir.glob(['spec/example_spec.rb[1:2:3:4]', 'spec/example_spec.rb:1'])
# => []

Workaround

Manually invoke the spec Rake task after overriding Rake::FileList::GLOB_PATTERN to not include the [ bracket.

rspec = Rake::Task['spec']

Rake::FileList::GLOB_PATTERN = %r{[*?\{]} # remove [ from GLOB_PATTERN so [1:2:3:4] patterns work
ENV['SPEC'] = 'spec/example_spec.rb[1:2]'

rspec.invoke

Solution

It may be easier to not use Rake::FileList here, since it's actually not doing much.

[pirj]

What if we returned "--pattern #{escape ENV['SPEC']}" instead of the file list there?

[JonRowe]

That would produce the wrong result, file list is parsing a string of filenames into an array here, its supposed to pass them through as is to the cmd but taking into account any globbing, its just a very old peice of code that was never updated to support the id syntax.

We do all the work to support that in configuration, we could "simply" split on a space and then sort [sorting is done for consistency here] but that would drop globbing support, so we would probably have to map over the result and/or check for the glob to expand then sort?

[ezekg]

Is there a reason SPEC should not be treated the same as a pattern?

rspec-core/lib/rspec/core/rake_task.rb

Lines 124 to 154 in f8c8880

	elsif String === pattern && !File.exist?(pattern)
	return if [*rspec_opts].any? { |opt| opt =~ /--pattern/ }
	"--pattern #{escape pattern}"
	else
	# Before RSpec 3.1, we used `FileList` to get the list of matched
	# files, and then pass that along to the `rspec` command. Starting
	# with 3.1, we prefer to pass along the pattern as-is to the `rspec`
	# command, for 3 reasons:
	#
	# * It's *much* less verbose to pass one `--pattern` option than a
	# long list of files.
	# * It ensures `task.pattern` and `--pattern` have the same
	# behavior.
	# * It fixes a bug, where
	# `task.pattern = pattern_that_matches_no_files` would run *all*
	# files because it would cause no pattern or file args to get
	# passed to `rspec`, which causes all files to get run.
	#
	# However, `FileList` is *far* more flexible than the `--pattern`
	# option. Specifically, it supports individual files and directories,
	# as well as arrays of files, directories and globs, as well as other
	# `FileList` objects.
	#
	# For backwards compatibility, we have to fall back to using FileList
	# if the user has passed a `pattern` option that will not work with
	# `--pattern`.
	#
	# TODO: consider deprecating support for this and removing it in
	# RSpec 4.
	FileList[pattern].sort.map { |file| escape file }
	end

The bug mentioned in this comment happens with SPEC as well.

[JonRowe]

Pattern generates a list of specs to consider the examples, SPEC was created to pass specific files within that list to rspec. Patterns don't support ids either FWIW.

[ezekg]

Could we partition based on the pattern? Here's a quick example partitioning by the :1 and [1:2:3] suffixes.

if ENV['SPEC']
+ patterns       = ENV['SPEC'].split
+ examples, rest = patterns.partition { _1.match?(/((\[\d+(:\d+)*\])|(:\d+))$/) }
+
- FileList[ENV['SPEC']].sort
+ (FileList[rest] + examples).sort
elsif String === pattern && !File.exist?(pattern) 
  return if [*rspec_opts].any? { |opt| opt =~ /--pattern/ } 
  "--pattern #{escape pattern}" 
else 
  # Before RSpec 3.1, we used `FileList` to get the list of matched 
  # files, and then pass that along to the `rspec` command. Starting 
  # with 3.1, we prefer to pass along the pattern as-is to the `rspec` 
  # command, for 3 reasons: 
  # 
  #   * It's *much* less verbose to pass one `--pattern` option than a 
  #     long list of files. 
  #   * It ensures `task.pattern` and `--pattern` have the same 
  #     behavior. 
  #   * It fixes a bug, where 
  #     `task.pattern = pattern_that_matches_no_files` would run *all* 
  #     files because it would cause no pattern or file args to get 
  #     passed to `rspec`, which causes all files to get run. 
  # 
  # However, `FileList` is *far* more flexible than the `--pattern` 
  # option. Specifically, it supports individual files and directories, 
  # as well as arrays of files, directories and globs, as well as other 
  # `FileList` objects. 
  # 
  # For backwards compatibility, we have to fall back to using FileList 
  # if the user has passed a `pattern` option that will not work with 
  # `--pattern`. 
  # 
  # TODO: consider deprecating support for this and removing it in 
  #   RSpec 4. 
  FileList[pattern].sort.map { |file| escape file } 
end 

[JonRowe]

We could look for id based filenames and pass them directly, and pass the rest of the string to file list yes.

As an aside thats not "the pattern" this file uses elsewhere, that is an accessor method containing either a custom pattern or the default which is essentially [with some extra escaping] spec/**/*_spec.rb

So this statement:

The bug mentioned in this comment happens with SPEC as well.

Is actually incorrect, as pattern would still be set even if SPEC= something_that_doesnt_match and infact:

 SPEC=spec/rspec/core/an_non_existant_spec.rb rake spec
...
LoadError:
  cannot load such file -- /Users/jon/Code/Libs/Ruby/rspec-core/spec/rspec/core/an_non_existant_spec.rb

In fact this is also why an id won't run any specs, nothing matches.

The bug described is talking about empty pattern options being ignored by the cli and using the default instead.

[JonRowe]

If you'd like to work on a PR to improve the pass through of ids feel free, not something I will have time for.

[ezekg]

Is actually incorrect, as pattern would still be set even if SPEC=something_that_doesnt_match and infact:

SPEC=spec/rspec/core/an_non_existant_spec.rb rake spec
...
LoadError:
  cannot load such file -- /Users/jon/Code/Libs/Ruby/rspec-core/spec/rspec/core/an_non_existant_spec

Just for clarity, I was referring to the following:

SPEC=spec/rspec/core/an_non_existant_spec.rb[1:2:3] rake spec

Using version 3.2.2, it runs everything when an example ID is present, instead of raising.

I'll look at opening a PR soon.

[JonRowe]

Ah interesting, I guess its the same cause but slightly different faceat of the issue, file list interprets the [...] part and produces a blank list, any pattern would still be set though. I definietly think finding specs matching the line / file syntax and splitting them out of the list is the way to go then, in RSpec 4 we may just remove usage of this to aline with the CLI