1.8.7/Jruby: handle --bisect in SPEC_OPTS by mutating environment

Open3.popen3 doesn't support passing a modified environment hash like
.capture2e. We can therefore either build a command string with an explicit
environment in it, or modify the current environment before invoking the
command.

While not mutating the current env would normally be preferable, I wasn't
confident I could do so correctly while accounting for shell escaping, spaces
etc.

I've therefore tried modifying the environment, and have provided a new custom
matcher that switches on the Open3 command in use.

Author: urbanautomaton

lib/rspec/core/bisect/runner.rb

@@ -72,36 +72,43 @@ 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(spec_opts_without_bisect, cmd).first
+            Open3.capture2e(bisect_environment_hash, cmd).first
           end
         else # for 1.8.7
           # :nocov:
           def run_command(cmd)
             out = err = nil
 
-            Open3.popen3(spec_opts_without_bisect, cmd) do |_, stdout, stderr|
+            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 spec_opts_without_bisect
+        def bisect_environment_hash
           if ENV.key?('SPEC_OPTS')
-            { 'SPEC_OPTS' => spec_opts_args_without_bisect.join(" ") }
+            { 'SPEC_OPTS' => spec_opts_without_bisect }
           else
             {}
           end
         end
 
-        def spec_opts_args_without_bisect
-          Shellwords.split(ENV.fetch('SPEC_OPTS', '')).reject do |arg|
-            arg =~ /^--bisect/
-          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

spec/rspec/core/bisect/runner_spec.rb

@@ -213,6 +213,27 @@ def repro_command_from(ids)
       open3_method = Open3.respond_to?(:capture2e) ? :capture2e : :popen3
       open3_method = :popen3 if RSpec::Support::Ruby.jruby?
 
+      RSpec::Matchers.define :invoke_command_with_env do |command, environment|
+        match do |actual|
+          if open3_method == :capture2e
+            actual.call
+            expect(Open3).to have_received(open3_method).with(environment, command)
+          elsif open3_method == :popen3
+            called_environment = nil
+            allow(Open3).to receive(open3_method) do
+              called_environment = ENV.to_hash.dup
+            end
+            actual.call
+            expect(Open3).to have_received(open3_method).with(command)
+            expect(called_environment).to include(environment)
+          end
+        end
+
+        def supports_block_expectations?
+          true
+        end
+      end
+
       before do
         allow(Open3).to receive(open3_method).and_return(
           [double("Exit Status"), double("Stdout/err")]
@@ -224,32 +245,33 @@ def repro_command_from(ids)
       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 invoke_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
-          with_env_vars 'SPEC_OPTS' => '--bisect --fail-fast' do
-            runner.original_results
-          end
-
-          expect(Open3).to have_received(open3_method).with(
-            { 'SPEC_OPTS' => '--fail-fast' },
-            a_string_including("--seed 1234")
+          expect {
+            with_env_vars 'SPEC_OPTS' => '--bisect --fail-fast' do
+              runner.original_results
+            end
+          }.to invoke_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
-          with_env_vars 'SPEC_OPTS' => '--bisect=verbose --fail-fast' do
-            runner.original_results
-          end
-
-          expect(Open3).to have_received(open3_method).with(
-            { 'SPEC_OPTS' => '--fail-fast' },
-            a_string_including("--seed 1234")
+          expect {
+            with_env_vars 'SPEC_OPTS' => '--bisect=verbose --fail-fast' do
+              runner.original_results
+            end
+          }.to invoke_command_with_env(
+            a_string_including("--seed 1234"),
+            { 'SPEC_OPTS' => '--fail-fast' }
           )
         end
       end