Merge pull request #1289 from rspec/fix-spec-helper-files-to-run

Fix for #1280

Author: myronmarston

features/command_line/init.feature

@@ -22,14 +22,36 @@ Feature: --init option
     Given I have a brand new project with no files
       And I have run `rspec --init`
      When I accept the recommended settings by removing `=begin` and `=end` from `spec/spec_helper.rb`
-      And I create a spec file with the following content:
-        """
+      And I create "spec/addition_spec.rb" with the following content:
+        """ruby
         RSpec.describe "Addition" do
           it "works" do
             expect(1 + 1).to eq(2)
           end
         end
         """
-      And I run `rspec`
+      And I create "spec/subtraction_spec.rb" with the following content:
+        """ruby
+        RSpec.describe "Subtraction" do
+          it "works" do
+            expect(1 - 1).to eq(0)
+          end
+        end
+        """
+
+     When I run `rspec`
+     Then the examples should all pass
+      And the output should not contain:
+        """
+        Addition
+          works
+        """
+
+     When I run `rspec spec/addition_spec.rb`
      Then the examples should all pass
+      And the output should contain:
+        """
+        Addition
+          works
+        """
 

features/step_definitions/additional_cli_steps.rb

@@ -73,7 +73,7 @@
   end
 end
 
-When /^I create a spec file with the following content:$/ do |content|
-  write_file("spec/example_spec.rb", content)
+When /^I create "([^"]*)" with the following content:$/ do |file_name, content|
+  write_file(file_name, content)
 end
 

lib/rspec/core/command_line.rb

@@ -4,7 +4,6 @@ class CommandLine
       def initialize(options, configuration=RSpec::configuration, world=RSpec::world)
         if Array === options
           options = ConfigurationOptions.new(options)
-          options.parse_options
         end
         @options       = options
         @configuration = configuration

lib/rspec/core/configuration.rb

@@ -231,7 +231,7 @@ def treat_symbols_as_metadata_keys_with_true_values=(value)
       # @private
       add_setting :include_or_extend_modules
       # @private
-      add_setting :files_to_run
+      attr_writer :files_to_run
       # @private
       add_setting :expecting_with_rspec
       # @private
@@ -248,7 +248,7 @@ def initialize
         @expectation_frameworks = []
         @include_or_extend_modules = []
         @mock_framework = nil
-        @files_to_run = []
+        @files_or_directories_to_run = []
         @color = false
         @pattern = '**/*_spec.rb'
         @failure_exit_code = 1
@@ -620,7 +620,12 @@ def profile_examples
       def files_or_directories_to_run=(*files)
         files = files.flatten
         files << default_path if (command == 'rspec' || Runner.running_in_drb?) && default_path && files.empty?
-        self.files_to_run = get_files_to_run(files)
+        @files_or_directories_to_run = files
+        @files_to_run = nil
+      end
+
+      def files_to_run
+        @files_to_run ||= get_files_to_run(@files_or_directories_to_run)
       end
 
       # Creates a method that delegates to `example` including the submitted
@@ -897,7 +902,7 @@ def safe_include(mod, host)
       end
 
       # @private
-      def setup_load_path_and_require(paths)
+      def requires=(paths)
         directories = ['lib', default_path].select { |p| File.directory? p }
         RSpec::Core::RubyProject.add_to_load_path(*directories)
         paths.each {|path| require path}

lib/rspec/core/configuration_options.rb

@@ -5,8 +5,6 @@ module RSpec
   module Core
     # @private
     class ConfigurationOptions
-      attr_reader :options
-
       def initialize(args)
         @args = args.map {|a|
           a.sub("default_path", "default-path").sub("line_number",  "line-number")
@@ -16,20 +14,17 @@ def initialize(args)
       def configure(config)
         config.filter_manager = filter_manager
 
-        config.libs = options[:libs] || []
-        config.setup_load_path_and_require(options[:requires] || [])
-
         process_options_into config
         load_formatters_into config
       end
 
-      def parse_options
-        @options = (file_options << command_line_options << env_options).
+      def options
+        @options ||= (file_options << command_line_options << env_options).
           each {|opts|
             filter_manager.include opts.delete(:inclusion_filter) if opts.has_key?(:inclusion_filter)
             filter_manager.exclude opts.delete(:exclusion_filter) if opts.has_key?(:exclusion_filter)
           }.
-          inject {|h, opts|
+          inject(:libs => [], :requires => []) {|h, opts|
             h.merge(opts) {|k, oldval, newval|
               [:libs, :requires].include?(k) ? oldval + newval : newval
             }
@@ -51,23 +46,44 @@ def filter_manager
         :line_numbers, :full_description, :full_backtrace, :tty
       ].to_set
 
-      UNPROCESSABLE_OPTIONS = [:libs, :formatters, :requires].to_set
+      UNPROCESSABLE_OPTIONS = [:formatters].to_set
 
       def force?(key)
         !UNFORCED_OPTIONS.include?(key)
       end
 
-      def order(keys, *ordered)
-        ordered.reverse.each do |key|
+      def order(keys)
+        OPTIONS_ORDER.reverse.each do |key|
           keys.unshift(key) if keys.delete(key)
         end
         keys
       end
 
+      OPTIONS_ORDER = [
+        # load paths depend on nothing, but must be set before `requires`
+        # to support load-path-relative requires.
+        :libs,
+
+        # `files_or_directories_to_run` uses `default_path` so it must be
+        # set before it.
+        :default_path,
+
+        # must be set before `requires` to support checking `config.files_to_run`
+        # from within `spec_helper.rb` when a `-rspec_helper` option is used.
+        :files_or_directories_to_run,
+
+        # In general, we want to require the specified files as early as possible.
+        # The `--require` option is specifically intended to allow early requires.
+        # For later requires, they can just put the require in their spec files, but
+        # `--require` provides a unique opportunity for users to instruct RSpec to
+        # load an extension file early for maximum flexibility.
+        :requires
+      ]
+
       def process_options_into(config)
         opts = options.reject { |k, _| UNPROCESSABLE_OPTIONS.include? k }
 
-        order(opts.keys, :default_path, :pattern).each do |key|
+        order(opts.keys).each do |key|
           force?(key) ? config.force(key => opts[key]) : config.__send__("#{key}=", opts[key])
         end
       end

lib/rspec/core/runner.rb

@@ -87,7 +87,6 @@ def self.trap_interrupt
       def self.run(args, err=$stderr, out=$stdout)
         trap_interrupt
         options = ConfigurationOptions.new(args)
-        options.parse_options
 
         if options.options[:drb]
           require 'rspec/core/drb_command_line'

spec/rspec/core/command_line_spec.rb

@@ -33,7 +33,7 @@ module RSpec::Core
     it "assigns ConfigurationOptions built from Array of options to @options" do
       config_options = ConfigurationOptions.new(%w[--color])
       command_line   = CommandLine.new(%w[--color])
-      expect(command_line.instance_eval { @options.options }).to eq(config_options.parse_options)
+      expect(command_line.instance_eval { @options.options }).to eq(config_options.options)
     end
 
     it "assigns submitted ConfigurationOptions to @options" do
@@ -106,9 +106,7 @@ def build_command_line *args
     end
 
     def build_config_options *args
-      options = ConfigurationOptions.new args
-      options.parse_options
-      options
+      ConfigurationOptions.new args
     end
   end
 end

spec/rspec/core/configuration_options_spec.rb

@@ -8,14 +8,13 @@
   it "warns when HOME env var is not set", :unless => (RUBY_PLATFORM == 'java') do
     without_env_vars 'HOME' do
       expect_warning_with_call_site(__FILE__, __LINE__ + 1)
-      RSpec::Core::ConfigurationOptions.new([]).parse_options
+      RSpec::Core::ConfigurationOptions.new([]).options
     end
   end
 
-  it "duplicates the arguments array" do
+  it "does not mutate the provided args array" do
     args = ['-e', 'some spec']
-    coo = RSpec::Core::ConfigurationOptions.new(args)
-    coo.parse_options
+    RSpec::Core::ConfigurationOptions.new(args).options
     expect(args).to eq(['-e', 'some spec'])
   end
 
@@ -26,40 +25,59 @@
       opts = config_options_object(*%w[--require a/path -I a/lib])
       config = double("config").as_null_object
       expect(config).to receive(:libs=).ordered
-      expect(config).to receive(:setup_load_path_and_require).ordered
+      expect(config).to receive(:requires=).ordered
       opts.configure(config)
     end
 
-    it "sends loads requires before loading specs" do
+    it "loads requires before loading specs" do
       opts = config_options_object(*%w[-rspec_helper])
-      config = double("config").as_null_object
-      expect(config).to receive(:setup_load_path_and_require).ordered
-      expect(config).to receive(:files_or_directories_to_run=).ordered
+      config = RSpec::Core::Configuration.new
+      expect(config).to receive(:requires=).ordered
+      expect(config).to receive(:get_files_to_run).ordered
       opts.configure(config)
+      config.files_to_run
     end
 
     it "sets up load path and requires before formatter" do
       opts = config_options_object(*%w[--require a/path -f a/formatter])
       config = double("config").as_null_object
-      expect(config).to receive(:setup_load_path_and_require).ordered
+      expect(config).to receive(:requires=).ordered
       expect(config).to receive(:add_formatter).ordered
       opts.configure(config)
     end
 
-    it "sends default_path before files_or_directories_to_run" do
+    it "sets default_path before loading specs" do
       opts = config_options_object(*%w[--default_path spec])
-      config = double("config").as_null_object
+      config = RSpec::Core::Configuration.new
+      expect(config).to receive(:force).with(:default_path => 'spec').ordered
+      expect(config).to receive(:get_files_to_run).ordered
+      opts.configure(config)
+      config.files_to_run
+    end
+
+    it "sets `files_or_directories_to_run` before `requires` so users can check `files_to_run` in a spec_helper loaded by `--require`" do
+      opts = config_options_object(*%w[--require spec_helper])
+      config = RSpec::Core::Configuration.new
+      expect(config).to receive(:files_or_directories_to_run=).ordered
+      expect(config).to receive(:requires=).ordered
+      opts.configure(config)
+    end
+
+    it "sets default_path before `files_or_directories_to_run` since it relies on it" do
+      opts = config_options_object(*%w[--default_path spec])
+      config = RSpec::Core::Configuration.new
       expect(config).to receive(:force).with(:default_path => 'spec').ordered
       expect(config).to receive(:files_or_directories_to_run=).ordered
       opts.configure(config)
     end
 
-    it "sends pattern before files_or_directories_to_run" do
+    it "sets pattern before loading specs" do
       opts = config_options_object(*%w[--pattern **/*.spec])
-      config = double("config").as_null_object
+      config = RSpec::Core::Configuration.new
       expect(config).to receive(:force).with(:pattern => '**/*.spec').ordered
-      expect(config).to receive(:files_or_directories_to_run=).ordered
+      expect(config).to receive(:get_files_to_run).ordered
       opts.configure(config)
+      config.files_to_run
     end
 
     it "assigns inclusion_filter" do
@@ -301,11 +319,12 @@
 
   describe "default_path" do
     it "gets set before files_or_directories_to_run" do
-      config = double("config").as_null_object
+      config = RSpec::Core::Configuration.new
       expect(config).to receive(:force).with(:default_path => 'foo').ordered
-      expect(config).to receive(:files_or_directories_to_run=).ordered
+      expect(config).to receive(:get_files_to_run).ordered
       opts = config_options_object("--default_path", "foo")
       opts.configure(config)
+      config.files_to_run
     end
   end