Skip to content

Commit

Permalink
Fix another regression in rake task pattern handling.
Browse files Browse the repository at this point in the history
This was reported by @aprescott: #1671 (comment).

The regression was originally introduced in #1653.
See also #1691 and #1692.
  • Loading branch information
myronmarston committed Sep 5, 2014
1 parent 60036a8 commit a2379d0
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 21 deletions.
10 changes: 10 additions & 0 deletions Changelog.md
@@ -1,3 +1,13 @@
### 3.1.1 Development
[Full Changelog](http://github.com/rspec/rspec-core/compare/v3.1.0...3-1-maintenance)

Bug Fixes:

* Fix a regression in rake task pattern handling, so that `rake_task.pattern = array`
works again. While we never intended to support array values (or even knew that worked!),
the implementation from 3.0 and earlier used `FileList` internally, which allows arrays.
The fix restores the old behavior. (Myron Marston, #1694)

### 3.1.0 / 2014-09-04
[Full Changelog](http://github.com/rspec/rspec-core/compare/v3.0.4...v3.1.0)

Expand Down
24 changes: 17 additions & 7 deletions lib/rspec/core/rake_task.rb
Expand Up @@ -114,15 +114,25 @@ def define(args, &task_block)
def file_inclusion_specification
if ENV['SPEC']
FileList[ ENV['SPEC']].sort
elsif File.exist?(pattern)
# The provided pattern is a directory or a file, not a file glob. Historically, this
# worked because `FileList[some_dir]` would return `[some_dir]` which would
# get passed to `rspec` and cause it to load files under that dir that match
# the default pattern. To continue working, we need to pass it on to `rspec`
# directly rather than treating it as a `--pattern` option.
elsif Array === pattern || File.exist?(pattern)
# 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.
#
# 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.
pattern.shellescape
FileList[pattern].sort.map(&:shellescape)
else
"--pattern #{pattern.shellescape}"
end
Expand Down
64 changes: 50 additions & 14 deletions spec/rspec/core/rake_task_spec.rb
Expand Up @@ -166,24 +166,60 @@ def self.it_configures_rspec_load_path(description, path_template)
specify_consistent_ordering_of_files_to_run('a/*.rb', Dir)
end

context "with a pattern that matches no files" do
it "runs nothing" do
task.pattern = 'a/*.no_match'
expect(loaded_files).to eq([])
context "with a pattern value" do
context "that matches no files" do
it "runs nothing" do
task.pattern = 'a/*.no_match'
expect(loaded_files).to eq([])
end
end
end

context "with a pattern value that is an existing directory, not a file glob" do
it "loads the spec files in that directory" do
task.pattern = "./spec/rspec/core/resources/acceptance"
expect(loaded_files).to eq(["./spec/rspec/core/resources/acceptance/foo_spec.rb"])
context "that is an existing directory, not a file glob" do
it "loads the spec files in that directory" do
task.pattern = "./spec/rspec/core/resources/acceptance"
expect(loaded_files).to eq(["./spec/rspec/core/resources/acceptance/foo_spec.rb"])
end
end
end

context "with a pattern value that is an existing file, not a file glob" do
it "loads the spec file" do
task.pattern = "./spec/rspec/core/resources/acceptance/foo_spec.rb"
expect(loaded_files).to eq(["./spec/rspec/core/resources/acceptance/foo_spec.rb"])
context "that is an existing file, not a file glob" do
it "loads the spec file" do
task.pattern = "./spec/rspec/core/resources/acceptance/foo_spec.rb"
expect(loaded_files).to eq(["./spec/rspec/core/resources/acceptance/foo_spec.rb"])
end
end

context "that is an array of existing files or directories, not a file glob" do
it "loads the specified spec files, and spec files from the specified directories" do
task.pattern = ["./spec/rspec/core/resources/acceptance", "./spec/rspec/core/resources/a_bar.rb"]

expect(loaded_files).to contain_exactly(
"./spec/rspec/core/resources/acceptance/foo_spec.rb",
"./spec/rspec/core/resources/a_bar.rb"
)
end
end

context "that is an array of globs relative to the current working dir" do
it "loads spec files that match any of the globs" do
task.pattern = ["./spec/rspec/core/resources/acceptance/*_spec.rb",
"./spec/rspec/core/resources/*_bar.rb"]

expect(loaded_files).to contain_exactly(
"./spec/rspec/core/resources/acceptance/foo_spec.rb",
"./spec/rspec/core/resources/a_bar.rb"
)
end
end

context "that is a mixture of file globs and individual files or dirs" do
it "loads all specified or matching files" do
task.pattern = ["./spec/rspec/core/resources/acceptance/*_spec.rb", "./spec/rspec/core/resources/a_bar.rb"]

expect(loaded_files).to contain_exactly(
"./spec/rspec/core/resources/acceptance/foo_spec.rb",
"./spec/rspec/core/resources/a_bar.rb"
)
end
end
end

Expand Down

0 comments on commit a2379d0

Please sign in to comment.