Support --bisect in SPEC_OPTS

Currently if RSpec is invoked with SPEC_OPTS="--bisect", an infinite number
of child processes is spawned. This is because the bisect feature works by
spawning child processes for each bisection run. In the normal cli usage case,
the --bisect argument is removed before the child processes are invoked. It
is not removed from the SPEC_OPTS variable, however, meaning each child
process believes it's starting a new bisection.

This commit allows Bisect::Runner to invoke its child processes with a
modified environment, removing --bisect if present. In modern rubies we can
use Open3.capture2e's optional environment argument - in older rubies and
JRuby we have to fall back on modifying the parent process's environment, then
restoring it after the child process has run.

Previously the Bisect::Runner was invoked during options parsing. However,
because SPEC_OPTS and the cli args are parsed by separate instances of the
parser, invoking the bisection during SPEC_OPTS parsing meant the cli args
were lost.

To address this I've made the OptionParser set a :bisect option, which is then
picked up in the main Runner to invoke the actual bisection. This has
increased the complexity of Runner.run, and there are other invocations
remaining in the OptionParser (e.g. --init and --help). These will be
refactored in a follow-up commit, so that the OptionParser sets a callable in
each case that can be called in the main Runner.

Author: urbanautomaton

lib/rspec/core/bisect/runner.rb

@@ -1,5 +1,6 @@
 RSpec::Support.require_rspec_core "shell_escape"
 require 'open3'
+require 'shellwords'
 
 module RSpec
   module Core
@@ -71,24 +72,45 @@ def run_locations(*capture_args)
         # https://github.com/jruby/jruby/issues/2766
         if Open3.respond_to?(:capture2e) && !RSpec::Support::Ruby.jruby?
           def run_command(cmd)
-            Open3.capture2e(cmd).first
+            Open3.capture2e(bisect_environment_hash, cmd).first
           end
         else # for 1.8.7
           # :nocov:
           def run_command(cmd)
             out = err = nil
 
+            original_spec_opts = ENV['SPEC_OPTS']
+            ENV['SPEC_OPTS'] = spec_opts_without_bisect
+
             Open3.popen3(cmd) do |_, stdout, stderr|
               # Reading the streams blocks until the process is complete
               out = stdout.read
               err = stderr.read
             end
 
             "Stdout:\n#{out}\n\nStderr:\n#{err}"
+          ensure
+            ENV['SPEC_OPTS'] = original_spec_opts
           end
           # :nocov:
         end
 
+        def bisect_environment_hash
+          if ENV.key?('SPEC_OPTS')
+            { 'SPEC_OPTS' => spec_opts_without_bisect }
+          else
+            {}
+          end
+        end
+
+        def spec_opts_without_bisect
+          Shellwords.join(
+            Shellwords.split(ENV.fetch('SPEC_OPTS', '')).reject do |arg|
+              arg =~ /^--bisect/
+            end
+          )
+        end
+
         def reusable_cli_options
           @reusable_cli_options ||= begin
             opts = original_cli_args_without_locations

lib/rspec/core/option_parser.rb

@@ -68,7 +68,7 @@ def parser(options)
 
         parser.on('--bisect[=verbose]', 'Repeatedly runs the suite in order to isolate the failures to the ',
                   '  smallest reproducible case.') do |argument|
-          bisect_and_exit(argument)
+          options[:bisect] = argument || true
         end
 
         parser.on('--[no-]fail-fast[=COUNT]', 'Abort the run after a certain number of failures (1 by default).') do |argument|
@@ -288,23 +288,6 @@ def initialize_project_and_exit
       exit
     end
 
-    def bisect_and_exit(argument)
-      RSpec::Support.require_rspec_core "bisect/coordinator"
-
-      success = Bisect::Coordinator.bisect_with(
-        original_args,
-        RSpec.configuration,
-        bisect_formatter_for(argument)
-      )
-
-      exit(success ? 0 : 1)
-    end
-
-    def bisect_formatter_for(argument)
-      return Formatters::BisectDebugFormatter if argument == "verbose"
-      Formatters::BisectProgressFormatter
-    end
-
     def print_version_and_exit
       puts RSpec::Core::Version::STRING
       exit

lib/rspec/core/runner.rb

@@ -73,11 +73,26 @@ def self.run(args, err=$stderr, out=$stdout)
           rescue DRb::DRbConnError
             err.puts "No DRb server is running. Running in local process instead ..."
           end
+        elsif options.options[:bisect]
+          RSpec::Support.require_rspec_core "bisect/coordinator"
+
+          success = Bisect::Coordinator.bisect_with(
+            args,
+            RSpec.configuration,
+            bisect_formatter_for(options.options[:bisect])
+          )
+
+          return success ? 0 : 1
         end
 
         new(options).run(err, out)
       end
 
+      def self.bisect_formatter_for(argument)
+        return Formatters::BisectDebugFormatter if argument == "verbose"
+        Formatters::BisectProgressFormatter
+      end
+
       def initialize(options, configuration=RSpec.configuration, world=RSpec.world)
         @options       = options
         @configuration = configuration

spec/integration/bisect_spec.rb

@@ -10,11 +10,10 @@ module RSpec::Core
 
     def bisect(cli_args, expected_status=nil)
       RSpec.configuration.output_stream = formatter_output
-      parser = Parser.new(cli_args + ["--bisect"])
-      expect(parser).to receive(:exit).with(expected_status) if expected_status
 
       expect {
-        parser.parse
+        status = RSpec::Core::Runner.run(cli_args + ["--bisect"])
+        expect(status).to eq(expected_status) if expected_status
       }.to avoid_outputting.to_stdout_from_any_process.and avoid_outputting.to_stderr_from_any_process
 
       normalize_durations(formatter_output.string)

spec/rspec/core/bisect/runner_spec.rb

@@ -213,19 +213,75 @@ def repro_command_from(ids)
       open3_method = Open3.respond_to?(:capture2e) ? :capture2e : :popen3
       open3_method = :popen3 if RSpec::Support::Ruby.jruby?
 
+      def called_environment
+        @called_environment
+      end
+
+      if open3_method == :capture2e
+        RSpec::Matchers.define :have_invoked_command_with_env do |command, environment|
+          match do |block|
+            block.call
+
+            expect(Open3).to have_received(open3_method).with(environment, command)
+          end
+
+          supports_block_expectations
+        end
+      elsif open3_method == :popen3
+        RSpec::Matchers.define :have_invoked_command_with_env do |command, environment|
+          match do |block|
+            block.call
+
+            expect(Open3).to have_received(open3_method).with(command)
+            expect(called_environment).to include(environment)
+          end
+
+          supports_block_expectations
+        end
+      end
+
       before do
-        allow(Open3).to receive(open3_method).and_return(
+        allow(Open3).to receive(open3_method) do
+          @called_environment = ENV.to_hash.dup
           [double("Exit Status"), double("Stdout/err")]
-        )
+        end
+
         allow(server).to receive(:capture_run_results) do |&block|
           block.call
           "the results"
         end
       end
 
       it "runs the suite with the original CLI options" do
-        runner.original_results
-        expect(Open3).to have_received(open3_method).with(a_string_including("--seed 1234"))
+        expect {
+          runner.original_results
+        }.to have_invoked_command_with_env(a_string_including("--seed 1234"), {})
+      end
+
+      context 'when --bisect is present in SPEC_OPTS' do
+        it "runs the suite with --bisect removed from the environment" do
+          expect {
+            with_env_vars 'SPEC_OPTS' => '--bisect --fail-fast' do
+              runner.original_results
+            end
+          }.to have_invoked_command_with_env(
+            a_string_including("--seed 1234"),
+            { 'SPEC_OPTS' => '--fail-fast' }
+          )
+        end
+      end
+
+      context 'when --bisect=verbose is present in SPEC_OPTS' do
+        it "runs the suite with --bisect removed from the environment" do
+          expect {
+            with_env_vars 'SPEC_OPTS' => '--bisect=verbose --fail-fast' do
+              runner.original_results
+            end
+          }.to have_invoked_command_with_env(
+            a_string_including("--seed 1234"),
+            { 'SPEC_OPTS' => '--fail-fast' }
+          )
+        end
       end
 
       it 'returns the run results' do