Pattern option with non-glob patterns behaves strange

[deivid-rodriguez]

Hei!

I'm having some trouble with the pattern option here: https://github.com/decidim/decidim/pull/1321. I'm including a list of two patterns, where one is a single path and the other one is a glob. It does what I expect only if I place the specific path first in the list

I wrote a couple of specs here to show the behavior. One fails and one passes.

diff --git a/features/command_line/pattern_option.feature b/features/command_line/pattern_option.feature
index baf5d93c..4c5fffa6 100644
--- a/features/command_line/pattern_option.feature
+++ b/features/command_line/pattern_option.feature
@@ -40,6 +40,14 @@ Feature: `--pattern` option
    When I run `rspec -P "**/*_test.rb,**/*_spec.rb"`
    Then the output should contain "3 examples, 0 failures"

+  Scenario: The `--pattern` flag can be used to pass in multiple patterns with the first having no globs, separated by commas
+   When I run `rspec -P "spec/example_test.rb,**/*_spec.rb"`
+   Then the output should contain "3 examples, 0 failures"
+
+  Scenario: The `--pattern` flag can be used to pass in multiple patterns with the last having no globs, separated by commas
+   When I run `rspec -P "**/*_spec.rb,spec/example_test.rb"`
+   Then the output should contain "3 examples, 0 failures"
+
   Scenario: The `--pattern` flag accepts shell style glob unions
    When I run `rspec -P "**/*_{test,spec}.rb"`
    Then the output should contain "3 examples, 0 failures"

Thanks a lot!

[JonRowe]

Sorry about this, this is a quirk due the nature of starting the pattern **, with other patterns we include the current working directory to allow you to use relative paths to it, but with ** with risk including vendored gems, tbh, the first should also exhibit this behaviour.

@myronmarston WDYT about changing this behaviour so it treats any path pattern segment starting with as `$folder_for_specs$/`

[myronmarston]

I don't entirely understand how ** is causing this but sure, that sounds reasonable.

[JonRowe]

It's due to this:

def paths_to_check(paths)
  return paths if pattern_might_load_specs_from_vendored_dirs?
  paths + [Dir.getwd]
end

def pattern_might_load_specs_from_vendored_dirs?
  pattern.split(File::SEPARATOR).first.include?('**')
end

Which is attempting to solve this:

 it "does not attempt to treat the pattern relative to `.` 
    if it uses `**` in the first path segment as that would
    cause it load specs from vendored gems" do
  Dir.chdir("./spec/rspec/core") do
  config.pattern = "**/*_spec.rb"
  assign_files_or_directories_to_run "spec" # default dir

  expect(config.files_to_run).to contain_files()
 end

[rhiroyuki]

Hi! I was able to recreate the author's issue using RSpec 3.8.0 and indeed, it still does the exact same thing he mentioned (if the pattern with ** is the first one then it won't run the spec file inside the spec folder).

If it's intended the way it's right now and no plans of changing it, do you think adding a warning about this behavior with ** would be a good call?

@JonRowe, do you think that this is an issue that could be closed? Any reasons to keep it open?

[JonRowe]

We discussed changing the behaviour so it would work for typical usages, but I would be open to a PR adding a warning about the behaviour instead

[rhiroyuki]

We discussed changing the behaviour so it would work for typical usages, but I would be open to a PR adding a warning about the behaviour instead

I can try adding the warning if you don't mind :+1:

[JonRowe]

Sure thing, let me know if you need any help