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

Allow adding a subclass of an already added formatter #2019

Merged
merged 2 commits into from
Jul 10, 2015
Merged

Allow adding a subclass of an already added formatter #2019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2b9a06
Allow adding a formatter class that extends from an already added for…
Jul 3, 2015
17fb14e
Updated formatter specs
Jul 9, 2015
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: 1 addition & 1 deletion lib/rspec/core/formatters.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def register(formatter, notifications)

def duplicate_formatter_exists?(new_formatter)
@formatters.any? do |formatter|
formatter.class === new_formatter && formatter.output == new_formatter.output
formatter.class == new_formatter.class && formatter.output == new_formatter.output
end
end

Expand Down
13 changes: 13 additions & 0 deletions spec/rspec/core/formatters_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ module RSpec::Core::Formatters
}.to change { loader.formatters.length }
end
end

context "When a custom formatter exists" do
specific_formatter = RSpec::Core::Formatters::JsonFormatter
generic_formatter = specific_formatter.superclass

before { loader.add generic_formatter, output }

it "adds a subclass of that formatter for the same output target" do
expect {
loader.add specific_formatter, output
}.to change { loader.formatters.length }
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd prefer this if the classes were declared as local variables / lets inline rather than using ones prepared in another file, especially as for this spec it's completely unnecessary to have an implementation.

e.g.

a_general_formatter = Struct.new(:output)
a_more_specific_formatter = Class.new(a_general_formatter)

before { loader.add a_general_formatter, output }

it "adds a subclass of that formatter for the same output target" do
  expect {
    loader.add a_more_specific_formatter, output
  }.to change { loader.formatters.length }
end

Copy link
Author

Choose a reason for hiding this comment

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

I've tried that, but it doesn't work. None of the formatters are added.

and if you do instead

generic_formatter = BaseFormatter
specific_formatter = Class.new(generic_formatter)

Only the generic formatter is added (looks like classes don't get added unless they've registered).

The following works though (and doesn't require the custom formatter classes)

specific_formatter = RSpec::Core::Formatters::JsonFormatter
generic_formatter = specific_formatter.superclass

I'll add those changes.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yep, you do need to register otherwise you're considered a legacy formatter, still the new way is better, thanks!

end
end
end

describe "#setup_default" do
Expand Down