Merge pull request #2507 from rspec/myron/bisect-formatter-refactor

Avoid using `RSpec.configuration` from the bisect coordinator.

Author: myronmarston

lib/rspec/core/bisect/coordinator.rb

@@ -15,47 +15,40 @@ module Bisect
       #   - Formatters::BisectProgressFormatter: provides progress updates
       #     to the user.
       class Coordinator
-        def self.bisect_with(original_cli_args, configuration, formatter)
-          new(original_cli_args, configuration, formatter).bisect
+        def self.bisect_with(original_cli_args, formatter)
+          new(original_cli_args, formatter).bisect
         end
 
-        def initialize(original_cli_args, configuration, formatter)
+        def initialize(original_cli_args, formatter)
           @shell_command = ShellCommand.new(original_cli_args)
-          @configuration = configuration
-          @formatter     = formatter
+          @notifier      = Bisect::Notifier.new(formatter)
         end
 
         def bisect
-          @configuration.add_formatter @formatter
+          repro = ShellRunner.start(@shell_command) do |runner|
+            minimizer = ExampleMinimizer.new(@shell_command, runner, @notifier)
 
-          reporter.close_after do
-            repro = ShellRunner.start(@shell_command) do |runner|
-              minimizer = ExampleMinimizer.new(@shell_command, runner, reporter)
-
-              gracefully_abort_on_sigint(minimizer)
-              minimizer.find_minimal_repro
-              minimizer.repro_command_for_currently_needed_ids
-            end
-
-            reporter.publish(:bisect_repro_command, :repro => repro)
+            gracefully_abort_on_sigint(minimizer)
+            minimizer.find_minimal_repro
+            minimizer.repro_command_for_currently_needed_ids
           end
 
+          @notifier.publish(:bisect_repro_command, :repro => repro)
+
           true
         rescue BisectFailedError => e
-          reporter.publish(:bisect_failed, :failure_explanation => e.message)
+          @notifier.publish(:bisect_failed, :failure_explanation => e.message)
           false
+        ensure
+          @notifier.publish(:close)
         end
 
       private
 
-        def reporter
-          @configuration.reporter
-        end
-
         def gracefully_abort_on_sigint(minimizer)
           trap('INT') do
             repro = minimizer.repro_command_for_currently_needed_ids
-            reporter.publish(:bisect_aborted, :repro => repro)
+            @notifier.publish(:bisect_aborted, :repro => repro)
             exit(1)
           end
         end

lib/rspec/core/bisect/example_minimizer.rb

@@ -7,13 +7,13 @@ module Bisect
       # Contains the core bisect logic. Searches for examples we can ignore by
       # repeatedly running different subsets of the suite.
       class ExampleMinimizer
-        attr_reader :shell_command, :runner, :reporter, :all_example_ids, :failed_example_ids
+        attr_reader :shell_command, :runner, :all_example_ids, :failed_example_ids
         attr_accessor :remaining_ids
 
-        def initialize(shell_command, runner, reporter)
+        def initialize(shell_command, runner, notifier)
           @shell_command = shell_command
           @runner        = runner
-          @reporter      = reporter
+          @notifier      = notifier
         end
 
         def find_minimal_repro
@@ -164,7 +164,7 @@ def abort_if_ordering_inconsistent(results)
         end
 
         def notify(*args)
-          reporter.publish(*args)
+          @notifier.publish(*args)
         end
       end
     end

lib/rspec/core/bisect/utilities.rb

@@ -11,6 +11,22 @@ def self.for_failed_spec_run(spec_output)
               spec_output)
         end
       end
+
+      # Wraps a `formatter` providing a simple means to notify it in place
+      # of an `RSpec::Core::Reporter`, without involving configuration in
+      # any way.
+      # @private
+      class Notifier
+        def initialize(formatter)
+          @formatter = formatter
+        end
+
+        def publish(event, *args)
+          return unless @formatter.respond_to?(event)
+          notification = Notifications::CustomNotification.for(*args)
+          @formatter.__send__(event, notification)
+        end
+      end
     end
   end
 end

lib/rspec/core/formatters/bisect_progress_formatter.rb

@@ -6,15 +6,6 @@ module Formatters
       # @private
       # Produces progress output while bisecting.
       class BisectProgressFormatter < BaseTextFormatter
-        # We've named all events with a `bisect_` prefix to prevent naming collisions.
-        Formatters.register self, :bisect_starting, :bisect_original_run_complete,
-                            :bisect_round_started, :bisect_individual_run_complete,
-                            :bisect_complete, :bisect_repro_command,
-                            :bisect_failed, :bisect_aborted,
-                            :bisect_round_ignoring_ids, :bisect_round_detected_multiple_culprits,
-                            :bisect_dependency_check_started, :bisect_dependency_check_passed,
-                            :bisect_dependency_check_failed
-
         def bisect_starting(notification)
           @round_count = 0
           options = notification.original_cli_args.join(' ')
@@ -88,13 +79,10 @@ def bisect_aborted(notification)
       end
 
       # @private
-      # Produces detailed debug output while bisecting. Used when
-      # bisect is performed while the `DEBUG_RSPEC_BISECT` ENV var is used.
-      # Designed to provide details for us when we need to troubleshoot bisect bugs.
+      # Produces detailed debug output while bisecting. Used when bisect is
+      # performed with `--bisect=verbose`. Designed to provide details for
+      # us when we need to troubleshoot bisect bugs.
       class BisectDebugFormatter < BisectProgressFormatter
-        Formatters.register self, :bisect_original_run_complete, :bisect_individual_run_start,
-                            :bisect_individual_run_complete, :bisect_round_ignoring_ids
-
         def bisect_original_run_complete(notification)
           output.puts " (#{Helpers.format_duration(notification.duration)})"
 

lib/rspec/core/invocations.rb

@@ -31,7 +31,6 @@ def call(options, _err, _out)
 
           success = RSpec::Core::Bisect::Coordinator.bisect_with(
             options.args,
-            RSpec.configuration,
             bisect_formatter_for(options.options[:bisect])
           )
 
@@ -41,8 +40,13 @@ def call(options, _err, _out)
         private
 
         def bisect_formatter_for(argument)
-          return Formatters::BisectDebugFormatter if argument == "verbose"
-          Formatters::BisectProgressFormatter
+          klass = if argument == "verbose"
+                    Formatters::BisectDebugFormatter
+                  else
+                    Formatters::BisectProgressFormatter
+                  end
+
+          klass.new(RSpec.configuration.output_stream)
         end
       end
 

spec/rspec/core/bisect/coordinator_spec.rb

@@ -18,8 +18,7 @@ def find_minimal_repro(output, formatter=Formatters::BisectProgressFormatter)
       allow(Bisect::Server).to receive(:run).and_yield(instance_double(Bisect::Server))
       allow(Bisect::ShellRunner).to receive(:new).and_return(fake_runner)
 
-      RSpec.configuration.output_stream = output
-      Bisect::Coordinator.bisect_with([], RSpec.configuration, formatter)
+      Bisect::Coordinator.bisect_with([], formatter.new(output))
     ensure
       RSpec.reset # so that RSpec.configuration.output_stream isn't closed
     end

spec/rspec/core/bisect/utilities_spec.rb

@@ -0,0 +1,26 @@
+require 'rspec/core/bisect/utilities'
+
+module RSpec::Core
+  RSpec.describe Bisect::Notifier do
+    class ExampleFormatterClass
+      def foo(notification); end
+    end
+
+    let(:formatter) { instance_spy(ExampleFormatterClass) }
+    let(:notifier) { Bisect::Notifier.new(formatter) }
+
+    it 'publishes events to the wrapped formatter' do
+      notifier.publish :foo, :length => 15, :width => 12
+
+      expect(formatter).to have_received(:foo).with(an_object_having_attributes(
+        :length => 15, :width => 12
+      ))
+    end
+
+    it 'does not publish events the formatter does not recognize' do
+      expect {
+        notifier.publish :unrecognized_event, :length => 15, :width => 12
+      }.not_to raise_error
+    end
+  end
+end

spec/rspec/core/invocations_spec.rb

@@ -92,8 +92,7 @@ def run_invocation
 
         expect(RSpec::Core::Bisect::Coordinator).to have_received(:bisect_with).with(
           args,
-          RSpec.configuration,
-          Formatters::BisectProgressFormatter
+          an_instance_of(Formatters::BisectProgressFormatter)
         )
       end
 
@@ -123,8 +122,7 @@ def run_invocation
 
           expect(RSpec::Core::Bisect::Coordinator).to have_received(:bisect_with).with(
             args,
-            RSpec.configuration,
-            Formatters::BisectDebugFormatter
+            an_instance_of(Formatters::BisectDebugFormatter)
           )
         end
       end