prevent formater loader registering duplicate listeners

Author: JonRowe

Changelog.md

@@ -70,6 +70,8 @@ Bug Fixes:
   where filtered to a particular location.  (Ben Axnick, #1963)
 * Fix time formatting logic so that it displays 70 seconds as "1 minute,
   10 seconds" rather than "1 minute, 1 second". (Paul Brennan, #1984)
+* Fix regression where the formatter loader would allow duplicate formatters.
+  (Jon Rowe, #1990)
 
 ### 3.2.3 / 2015-04-06
 [Full Changelog](http://github.com/rspec/rspec-core/compare/v3.2.2...v3.2.3)

lib/rspec/core/formatters.rb

@@ -143,10 +143,10 @@ def add(formatter_to_use, *paths)
 
       if !Loader.formatters[formatter_class].nil?
         formatter = formatter_class.new(*args)
-        @reporter.register_listener formatter, *notifications_for(formatter_class)
+        register formatter, notifications_for(formatter_class)
       elsif defined?(RSpec::LegacyFormatters)
         formatter = RSpec::LegacyFormatters.load_formatter formatter_class, *args
-        @reporter.register_listener formatter, *formatter.notifications
+        register formatter, formatter.notifications
       else
         call_site = "Formatter added at: #{::RSpec::CallerFilter.first_non_rspec_line}"
 
@@ -161,10 +161,7 @@ def add(formatter_to_use, *paths)
           |
           |#{call_site}
         WARNING
-        return
       end
-      @formatters << formatter unless duplicate_formatter_exists?(formatter)
-      formatter
     end
 
   private
@@ -176,6 +173,13 @@ def find_formatter(formatter_to_use)
                             "maybe you meant 'documentation' or 'progress'?.")
     end
 
+    def register(formatter, notifications)
+      return if duplicate_formatter_exists?(formatter)
+      @reporter.register_listener formatter, *notifications
+      @formatters << formatter
+      formatter
+    end
+
     def duplicate_formatter_exists?(new_formatter)
       @formatters.any? do |formatter|
         formatter.class === new_formatter && formatter.output == new_formatter.output

spec/rspec/core/formatters_spec.rb

@@ -41,7 +41,7 @@ module RSpec::Core::Formatters
 
       context "when a legacy formatter is added with RSpec::LegacyFormatters" do
         formatter_class = Struct.new(:output)
-        let(:formatter) { double "formatter", :notifications => notifications }
+        let(:formatter) { double "formatter", :notifications => notifications, :output => output }
         let(:notifications) { [:a, :b, :c] }
 
         before do
@@ -57,6 +57,14 @@ module RSpec::Core::Formatters
           expect(reporter).to receive(:register_listener).with(formatter, *notifications)
           loader.add formatter_class, output
         end
+
+        it "will ignore duplicate legacy formatters" do
+          loader.add formatter_class, output
+          expect(reporter).to_not receive(:register_listener)
+          expect {
+            loader.add formatter_class, output
+          }.not_to change { loader.formatters.length }
+        end
       end
 
       context "when a legacy formatter is added without RSpec::LegacyFormatters" do
@@ -117,6 +125,7 @@ module RSpec::Core::Formatters
         before { loader.add :documentation, output }
 
         it "doesn't add the formatter for the same output target" do
+          expect(reporter).to_not receive(:register_listener)
           expect {
             loader.add :documentation, output
           }.not_to change { loader.formatters.length }