Merge pull request #2503 from rspec/myron/bisect-forker-prep-refactorings

Bisect forker prep refactorings

Author: myronmarston

lib/rspec/core/bisect/coordinator.rb

@@ -1,16 +1,16 @@
-RSpec::Support.require_rspec_core "bisect/server"
-RSpec::Support.require_rspec_core "bisect/runner"
+RSpec::Support.require_rspec_core "bisect/shell_command"
+RSpec::Support.require_rspec_core "bisect/shell_runner"
 RSpec::Support.require_rspec_core "bisect/example_minimizer"
+RSpec::Support.require_rspec_core "bisect/utilities"
 RSpec::Support.require_rspec_core "formatters/bisect_progress_formatter"
 
 module RSpec
   module Core
     module Bisect
       # @private
       # The main entry point into the bisect logic. Coordinates among:
-      #   - Bisect::Server: Receives suite results.
-      #   - Bisect::Runner: Runs a set of examples and directs the results
-      #     to the server.
+      #   - Bisect::ShellCommand: Generates shell commands to run spec subsets
+      #   - Bisect::ShellRunner: Runs a set of examples and returns the results.
       #   - Bisect::ExampleMinimizer: Contains the core bisect logic.
       #   - Formatters::BisectProgressFormatter: provides progress updates
       #     to the user.
@@ -20,18 +20,17 @@ def self.bisect_with(original_cli_args, configuration, formatter)
         end
 
         def initialize(original_cli_args, configuration, formatter)
-          @original_cli_args = original_cli_args
-          @configuration     = configuration
-          @formatter         = formatter
+          @shell_command = ShellCommand.new(original_cli_args)
+          @configuration = configuration
+          @formatter     = formatter
         end
 
         def bisect
           @configuration.add_formatter @formatter
 
           reporter.close_after do
-            repro = Server.run do |server|
-              runner    = Runner.new(server, @original_cli_args)
-              minimizer = ExampleMinimizer.new(runner, reporter)
+            repro = ShellRunner.start(@shell_command) do |runner|
+              minimizer = ExampleMinimizer.new(@shell_command, runner, reporter)
 
               gracefully_abort_on_sigint(minimizer)
               minimizer.find_minimal_repro

lib/rspec/core/bisect/example_minimizer.rb

@@ -1,16 +1,19 @@
+RSpec::Support.require_rspec_core "bisect/utilities"
+
 module RSpec
   module Core
     module Bisect
       # @private
       # Contains the core bisect logic. Searches for examples we can ignore by
       # repeatedly running different subsets of the suite.
       class ExampleMinimizer
-        attr_reader :runner, :reporter, :all_example_ids, :failed_example_ids
+        attr_reader :shell_command, :runner, :reporter, :all_example_ids, :failed_example_ids
         attr_accessor :remaining_ids
 
-        def initialize(runner, reporter)
-          @runner   = runner
-          @reporter = reporter
+        def initialize(shell_command, runner, reporter)
+          @shell_command = shell_command
+          @runner        = runner
+          @reporter      = reporter
         end
 
         def find_minimal_repro
@@ -82,7 +85,7 @@ def currently_needed_ids
         end
 
         def repro_command_for_currently_needed_ids
-          return runner.repro_command_from(currently_needed_ids) if remaining_ids
+          return shell_command.repro_command_from(currently_needed_ids) if remaining_ids
           "(Not yet enough information to provide any repro command)"
         end
 
@@ -108,7 +111,7 @@ def example_range(ids)
         end
 
         def prep
-          notify(:bisect_starting, :original_cli_args => runner.original_cli_args)
+          notify(:bisect_starting, :original_cli_args => shell_command.original_cli_args)
 
           _, duration = track_duration do
             original_results    = runner.original_results
@@ -135,7 +138,7 @@ def get_expected_failures_for?(ids)
           ids_to_run = ids + failed_example_ids
           notify(
             :bisect_individual_run_start,
-            :command => runner.repro_command_from(ids_to_run),
+            :command => shell_command.repro_command_from(ids_to_run),
             :ids_to_run => ids_to_run
           )
 

lib/rspec/core/bisect/server.rb

@@ -1,13 +1,11 @@
 require 'drb/drb'
 require 'drb/acl'
+RSpec::Support.require_rspec_core "bisect/utilities"
 
 module RSpec
   module Core
     # @private
     module Bisect
-      # @private
-      BisectFailedError = Class.new(StandardError)
-
       # @private
       # A DRb server that receives run results from a separate RSpec process
       # started by the bisect process.
@@ -27,7 +25,7 @@ def capture_run_results(files_or_directories_to_run=[], expected_failures=[])
           run_output = yield
 
           if latest_run_results.nil? || latest_run_results.all_example_ids.empty?
-            raise_bisect_failed(run_output)
+            raise BisectFailedError.for_failed_spec_run(run_output)
           end
 
           latest_run_results
@@ -49,21 +47,14 @@ def drb_port
           @drb_port ||= Integer(@drb.uri[/\d+$/])
         end
 
-        # Fetched via DRb by the BisectFormatter to determine when to abort.
+        # Fetched via DRb by the BisectDRbFormatter to determine when to abort.
         attr_accessor :expected_failures
 
-        # Set via DRb by the BisectFormatter with the results of the run.
+        # Set via DRb by the BisectDRbFormatter with the results of the run.
         attr_accessor :latest_run_results
 
         # Fetched via DRb to tell clients which files to run
         attr_accessor :files_or_directories_to_run
-
-      private
-
-        def raise_bisect_failed(run_output)
-          raise BisectFailedError, "Failed to get results from the spec " \
-                "run. Spec run output:\n\n#{run_output}"
-        end
       end
     end
   end

lib/rspec/core/bisect/runner.rb renamed to lib/rspec/core/bisect/shell_command.rb

@@ -1,33 +1,27 @@
 RSpec::Support.require_rspec_core "shell_escape"
-require 'open3'
 require 'shellwords'
 
 module RSpec
   module Core
     module Bisect
-      # Provides an API to run the suite for a set of locations, using
-      # the given bisect server to capture the results.
+      # Provides an API to generate shell commands to run the suite for a
+      # set of locations, using the given bisect server to capture the results.
       # @private
-      class Runner
+      class ShellCommand
         attr_reader :original_cli_args
 
-        def initialize(server, original_cli_args)
-          @server            = server
+        def initialize(original_cli_args)
           @original_cli_args = original_cli_args.reject { |arg| arg.start_with?("--bisect") }
         end
 
-        def run(locations)
-          run_locations(locations, original_results.failed_example_ids)
-        end
-
-        def command_for(locations)
+        def command_for(locations, server)
           parts = []
 
           parts << RUBY << load_path
           parts << open3_safe_escape(RSpec::Core.path_to_executable)
 
-          parts << "--format"   << "bisect"
-          parts << "--drb-port" << @server.drb_port
+          parts << "--format"   << "bisect-drb"
+          parts << "--drb-port" << server.drb_port
 
           parts.concat(reusable_cli_options)
           parts.concat(locations.map { |l| open3_safe_escape(l) })
@@ -46,8 +40,24 @@ def repro_command_from(locations)
           parts.join(" ")
         end
 
-        def original_results
-          @original_results ||= run_locations(original_locations)
+        def original_locations
+          parsed_original_cli_options.fetch(:files_or_directories_to_run)
+        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
 
       private
@@ -63,61 +73,12 @@ def original_results
           alias open3_safe_escape escape
         end
 
-        def run_locations(*capture_args)
-          @server.capture_run_results(*capture_args) do
-            run_command command_for([])
-          end
-        end
-
-        # `Open3.capture2e` does not work on JRuby:
-        # https://github.com/jruby/jruby/issues/2766
-        if Open3.respond_to?(:capture2e) && !RSpec::Support::Ruby.jruby?
-          def run_command(cmd)
-            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 environment_repro_parts
           bisect_environment_hash.map do |k, v|
             %Q(#{k}="#{v}")
           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
@@ -146,10 +107,6 @@ def parsed_original_cli_options
           @parsed_original_cli_options ||= Parser.parse(@original_cli_args)
         end
 
-        def original_locations
-          parsed_original_cli_options.fetch(:files_or_directories_to_run)
-        end
-
         def load_path
           @load_path ||= "-I#{$LOAD_PATH.map { |p| open3_safe_escape(p) }.join(':')}"
         end

lib/rspec/core/bisect/shell_runner.rb

@@ -0,0 +1,69 @@
+require 'open3'
+RSpec::Support.require_rspec_core "bisect/server"
+
+module RSpec
+  module Core
+    module Bisect
+      # Provides an API to run the suite for a set of locations, using
+      # the given bisect server to capture the results.
+      #
+      # Sets of specs are run by shelling out.
+      # @private
+      class ShellRunner
+        def self.start(shell_command)
+          Server.run do |server|
+            yield new(server, shell_command)
+          end
+        end
+
+        def initialize(server, shell_command)
+          @server        = server
+          @shell_command = shell_command
+        end
+
+        def run(locations)
+          run_locations(locations, original_results.failed_example_ids)
+        end
+
+        def original_results
+          @original_results ||= run_locations(@shell_command.original_locations)
+        end
+
+      private
+
+        def run_locations(*capture_args)
+          @server.capture_run_results(*capture_args) do
+            run_command @shell_command.command_for([], @server)
+          end
+        end
+
+        # `Open3.capture2e` does not work on JRuby:
+        # https://github.com/jruby/jruby/issues/2766
+        if Open3.respond_to?(:capture2e) && !RSpec::Support::Ruby.jruby?
+          def run_command(cmd)
+            Open3.capture2e(@shell_command.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'] = @shell_command.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
+      end
+    end
+  end
+end

lib/rspec/core/bisect/utilities.rb

@@ -0,0 +1,16 @@
+module RSpec
+  module Core
+    module Bisect
+      # @private
+      ExampleSetDescriptor = Struct.new(:all_example_ids, :failed_example_ids)
+
+      # @private
+      class BisectFailedError < StandardError
+        def self.for_failed_spec_run(spec_output)
+          new("Failed to get results from the spec run. Spec run output:\n\n" +
+              spec_output)
+        end
+      end
+    end
+  end
+end

lib/rspec/core/configuration.rb

@@ -877,11 +877,11 @@ def full_description
       # @overload add_formatter(formatter)
       # @overload add_formatter(formatter, output)
       #
-      # @param formatter [Class, String] formatter to use. Can be any of the
+      # @param formatter [Class, String, Object] formatter to use. Can be any of the
       #   string values supported from the CLI (`p`/`progress`,
-      #   `d`/`doc`/`documentation`, `h`/`html`, or `j`/`json`) or any
+      #   `d`/`doc`/`documentation`, `h`/`html`, or `j`/`json`), any
       #   class that implements the formatter protocol and has registered
-      #   itself with RSpec as a formatter.
+      #   itself with RSpec as a formatter, or a formatter instance.
       # @param output [String, IO] where the formatter will write its output.
       #   Can be an IO object or a string path to a file. If not provided,
       #   the configured `output_stream` (`$stdout`, by default) will be used.