-
-
Notifications
You must be signed in to change notification settings - Fork 753
Avoid using RSpec.configuration
from the bisect coordinator.
#2507
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure I follow why all this goes away? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you answered your own question above, but for posterity: previously, this was a "real" formatter that used with the Now that we are no longer using the reporter to publish events to the formatter during bisection runs, we don't have to register all these events. Instead, the new In general, we've learned that |
||
|
||
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)})" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we only used the config to add a formatter. Instead now we just call the formatter directly, right?
So that probably explains why we don't need to register all the events below. This also implies that previously I would have been able to hook in my own formatter to bisect events, where as now I ... can't? Guess it depends where the
formatter
comes from that's passed into the initializer.All LGTM, I figure you have this in your head, just trying to see if I'm following along :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used the config for 2 things:
reporter
, which we used to publish events to the formatterNow we just publish events to the formatter directly, without config and the reporter mediating access.
Yep!
In theory, yes, you could have, but we did not provide any way for users to specify a custom bisect formatter, so in practice, a user would have had to monkey patch RSpec to make it use their custom formatter for bisect events. So we aren't breaking any public APIs here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for extra explanation!