Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Add fallback formatter for #message #1980

Merged
merged 5 commits into from
Jun 2, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ Enhancements:
`aggregate_failures` feature to allow multiple failures in an example
and list them all, rather than aborting on the first failure. (Myron
Marston, #1946)
* When no formatter implements #message add a fallback to prevent those
messages being lost. (Jon Rowe, #1980)

Bug Fixes:

Expand Down
17 changes: 11 additions & 6 deletions lib/rspec/core/formatters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@
# @see RSpec::Core::Formatters::BaseTextFormatter
# @see RSpec::Core::Reporter
module RSpec::Core::Formatters
autoload :DocumentationFormatter, 'rspec/core/formatters/documentation_formatter'
autoload :HtmlFormatter, 'rspec/core/formatters/html_formatter'
autoload :ProgressFormatter, 'rspec/core/formatters/progress_formatter'
autoload :ProfileFormatter, 'rspec/core/formatters/profile_formatter'
autoload :JsonFormatter, 'rspec/core/formatters/json_formatter'
autoload :BisectFormatter, 'rspec/core/formatters/bisect_formatter'
autoload :DocumentationFormatter, 'rspec/core/formatters/documentation_formatter'
autoload :HtmlFormatter, 'rspec/core/formatters/html_formatter'
autoload :FallbackMessageFormatter, 'rspec/core/formatters/fallback_message_formatter'
autoload :ProgressFormatter, 'rspec/core/formatters/progress_formatter'
autoload :ProfileFormatter, 'rspec/core/formatters/profile_formatter'
autoload :JsonFormatter, 'rspec/core/formatters/json_formatter'
autoload :BisectFormatter, 'rspec/core/formatters/bisect_formatter'

# Register the formatter class
# @param formatter_class [Class] formatter class to register
Expand Down Expand Up @@ -121,6 +122,10 @@ def setup_default(output_stream, deprecation_stream)
add DeprecationFormatter, deprecation_stream, output_stream
end

unless existing_formatter_implements?(:message)
add FallbackMessageFormatter, output_stream
end

return unless RSpec.configuration.profile_examples? && !existing_formatter_implements?(:dump_profile)

add RSpec::Core::Formatters::ProfileFormatter, output_stream
Expand Down
28 changes: 28 additions & 0 deletions lib/rspec/core/formatters/fallback_message_formatter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module RSpec
module Core
module Formatters
# @api private
# Formatter for providing message output as a fallback when no other
# profiler implements #message
class FallbackMessageFormatter
Formatters.register self, :message

def initialize(output)
@output = output
end

# @private
attr_reader :output

# @api public
#
# Used by the reporter to send messages to the output stream.
#
# @param notification [MessageNotification] containing message
def message(notification)
output.puts notification.message
end
end
end
end
end
18 changes: 18 additions & 0 deletions spec/rspec/core/formatters/fallback_message_formatter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
require 'rspec/core/reporter'
require 'rspec/core/formatters/fallback_message_formatter'

module RSpec::Core::Formatters
RSpec.describe FallbackMessageFormatter do
include FormatterSupport

describe "#message" do
it 'writes the message to the output' do
expect {
send_notification :message, message_notification('Custom Message')
}.to change { formatter_output.string }.
from(excluding 'Custom Message').
to(including 'Custom Message')
end
end
end
end
57 changes: 42 additions & 15 deletions spec/rspec/core/formatters_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,28 +130,55 @@ module RSpec::Core::Formatters
end
end

describe "#setup_default", "with profiling enabled" do
describe "#setup_default" do
let(:setup_default) { loader.setup_default output, output }

before do
allow(RSpec.configuration).to receive(:profile_examples?) { true }
context "with a formatter that implements #message" do
it 'doesnt add a fallback formatter' do
allow(reporter).to receive(:registered_listeners).with(:message) { [:json] }
setup_default
expect(loader.formatters).to exclude(
an_instance_of ::RSpec::Core::Formatters::FallbackMessageFormatter
)
end
end

context "without an existing profile formatter" do
it "will add the profile formatter" do
allow(reporter).to receive(:registered_listeners).with(:dump_profile) { [] }
setup_default
expect(loader.formatters.last).to be_a ::RSpec::Core::Formatters::ProfileFormatter
context "without a formatter that implements #message" do
it 'adds a fallback for message output' do
allow(reporter).to receive(:registered_listeners).with(:message) { [] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to stub this here? Isn't the reporter instance here a bare reporter instance that doesn't have any listeners registered?

expect {
setup_default
}.to change { loader.formatters }.
from( excluding an_instance_of ::RSpec::Core::Formatters::FallbackMessageFormatter ).
to( including an_instance_of ::RSpec::Core::Formatters::FallbackMessageFormatter )
end
end

context "when a formatter that implement #dump_profile is added" do
it "wont add the profile formatter" do
allow(reporter).to receive(:registered_listeners).with(:dump_profile) { [:json] }
setup_default
expect(
loader.formatters.map(&:class)
).to_not include ::RSpec::Core::Formatters::ProfileFormatter
context "with profiling enabled" do
before do
allow(reporter).to receive(:registered_listeners).with(:message) { [:json] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems brittle to stub reporter.registered_listeners when it's not a public API. Why not use reporter.register_listener(spy, :message)? That uses a public API (which is going to be more stable as it's subject to SemVer!), doesn't fake things out unnecessarily, and won't have problems with the fact that :json isn't actually a listener since spy will gladly no-op listen to any message :).

Or do you see an advantage to stubbing the reporter here over just registering a listener?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not a real reporter, but an instance_double.

allow(RSpec.configuration).to receive(:profile_examples?) { true }
end

context "without an existing profile formatter" do
it "will add the profile formatter" do
allow(reporter).to receive(:registered_listeners).with(:dump_profile) { [] }
expect {
setup_default
}.to change { loader.formatters }.
from( excluding an_instance_of ::RSpec::Core::Formatters::ProfileFormatter ).
to( including an_instance_of ::RSpec::Core::Formatters::ProfileFormatter )
end
end

context "when a formatter that implement #dump_profile is added" do
it "wont add the profile formatter" do
allow(reporter).to receive(:registered_listeners).with(:dump_profile) { [:json] }
setup_default
expect(
loader.formatters.map(&:class)
).to_not include ::RSpec::Core::Formatters::ProfileFormatter
end
end
end
end
Expand Down
1 change: 1 addition & 0 deletions spec/support/matchers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,5 @@ def failure_reason(example)

RSpec::Matchers.define_negated_matcher :avoid_outputting, :output
RSpec::Matchers.define_negated_matcher :exclude, :include
RSpec::Matchers.define_negated_matcher :excluding, :include
RSpec::Matchers.define_negated_matcher :avoid_changing, :change