Merge pull request #1309 from rspec/formatter-regression-tests-prep-f…

…or-merge

Formatter regression tests prep for merge

cucumber.yml

@@ -3,4 +3,4 @@ exclusions = []
 exclusions << ' --tags ~@no-jruby' if RUBY_PLATFORM == 'java'
 %>
 default: --require features --strict --format progress --tags ~@wip<%= exclusions.join %> features
-wip:     --require features --tags @wip:3 --wip features
+wip:     --require features --tags @wip:30 --wip features

features/formatters/regression_tests.feature

@@ -0,0 +1,95 @@
+Feature: Regression tests for legacy custom formatters
+
+  Background:
+    Given a file named ".rspec" with:
+      """
+      --require spec_helper
+      """
+      And a file named "spec/spec_helper.rb" with:
+      """
+      RSpec.configure do |rspec|
+        rspec.after(:suite) do
+          puts rspec.formatters.map(&:class).inspect
+        end
+      end
+      """
+      And a file named "spec/passing_and_failing_spec.rb" with:
+      """ruby
+      RSpec.describe "Some examples" do
+        it "passes" do
+          expect(1).to eq(1)
+        end
+
+        it "fails" do
+          expect(1).to eq(2)
+        end
+
+        context "nested" do
+          it "passes" do
+            expect(1).to eq(1)
+          end
+
+          it "fails" do
+            expect(1).to eq(2)
+          end
+        end
+      end
+      """
+      And a file named "spec/pending_spec.rb" with:
+      """ruby
+      RSpec.describe "Some pending examples" do
+        context "pending" do
+          it "is reported as pending" do
+            pending { expect(1).to eq(2) }
+          end
+
+          it "is reported as failing" do
+            pending { expect(1).to eq(1) }
+          end
+        end
+      end
+      """
+
+  Scenario: Use fuubar formatter
+    When I run `rspec --format Fuubar`
+    Then the output should contain "Progress: |============"
+     And the output should contain "6 examples, 3 failures, 1 pending"
+     And the output should contain "The Fuubar formatter uses the deprecated formatter interface"
+     But the output should not contain any error backtraces
+     And the output should not contain "ProgressFormatter"
+
+  Scenario: Use rspec-instafail formatter
+    When I run `rspec --format RSpec::Instafail`
+    Then the output should contain "6 examples, 3 failures, 1 pending"
+     And the output should contain "The RSpec::Instafail formatter uses the deprecated formatter interface"
+     But the output should not contain any error backtraces
+     And the output should not contain "ProgressFormatter"
+
+  Scenario: Use rspec-extra-formatters JUnit formatter
+    When I run `rspec --require rspec-extra-formatters --format JUnitFormatter`
+    Then the output should contain:
+      """
+      <testsuite errors="0" failures="3" skipped="1" tests="6"
+      """
+     And the output should contain "The JUnitFormatter formatter uses the deprecated formatter interface"
+     But the output should not contain any error backtraces
+     And the output should not contain "ProgressFormatter"
+
+  @wip @announce
+  Scenario: Use rspec-extra-formatters Tap formatter
+    When I run `rspec --require rspec-extra-formatters --format TapFormatter`
+    Then the output should contain "TAP version 13"
+     And the output should contain "The TapFormatter formatter uses the deprecated formatter interface"
+     But the output should not contain any error backtraces
+     And the output should not contain "ProgressFormatter"
+
+  @wip @announce
+  Scenario: Use rspec-spinner formatter
+    When I run `rspec --require rspec_spinner --format RspecSpinner::Spinner`
+    Then the output should contain "TBD"
+
+  @wip @announce
+  Scenario: Use nyancat formatter
+    When I run `rspec --format NyanCatFormatter`
+    Then the output should contain "TBD"
+

features/step_definitions/additional_cli_steps.rb

@@ -41,6 +41,10 @@
   expect(normalized_output).to match(regexp(partial_output))
 end
 
+Then /^the output should not contain any error backtraces$/ do
+  step %q{the output should not contain "lib/rspec/core"}
+end
+
 # This step can be generalized if it's ever used to test other colors
 Then /^the failing example is printed in magenta$/ do
   # \e[35m = enable magenta

lib/rspec/core/formatters.rb

@@ -96,17 +96,22 @@ def setup_default(output_stream, deprecation_stream)
     # @api private
     def add(formatter_to_use, *paths)
       formatter_class = find_formatter(formatter_to_use)
-      formatter = formatter_class.new(*paths.map {|p| String === p ? file_at(p) : p})
+
+      args = paths.map { |p| String === p ? file_at(p) : p }
 
       if !Loader.formatters[formatter_class].nil?
+        formatter = formatter_class.new(*args)
         @reporter.register_listener formatter, *notifications_for(formatter_class)
       else
-        RSpec.warn_deprecation "The #{formatter.class} formatter uses the deprecated formatter interface.\n Formatter added at: #{::RSpec::CallerFilter.first_non_rspec_line}"
-        formatter = LegacyFormatter.new(formatter)
+        formatter = LegacyFormatter.new(formatter_class, *args)
         @reporter.register_listener formatter, *formatter.notifications
       end
 
       @formatters << formatter unless duplicate_formatter_exists?(formatter)
+      if formatter.is_a?(LegacyFormatter)
+        RSpec.warn_deprecation "The #{formatter_class} formatter uses the deprecated formatter interface.\n Formatter added at: #{::RSpec::CallerFilter.first_non_rspec_line}"
+      end
+
       formatter
     end
 

lib/rspec/core/formatters/legacy_formatter.rb

@@ -88,16 +88,19 @@ def stop
           end
         end
 
+        # @api private
+        attr_reader :formatter
+
         # @api public
         #
         # @param formatter
-        def initialize(oldstyle_formatter)
-          @formatter = oldstyle_formatter
-          if @formatter.class.ancestors.include?(BaseFormatter)
-            @formatter.class.class_eval do
+        def initialize(formatter_class, *args)
+          if formatter_class.ancestors.include?(BaseFormatter)
+            formatter_class.class_eval do
               include LegacyInterface
             end
           end
+          @formatter = formatter_class.new(*args)
         end
 
         # @api public

rspec-core.gemspec

@@ -46,8 +46,14 @@ Gem::Specification.new do |s|
   s.add_development_dependency "nokogiri", "1.5.2"
   s.add_development_dependency "coderay",  "~> 1.0.9"
 
-
   s.add_development_dependency "mocha",    "~> 0.13.0"
   s.add_development_dependency "rr",       "~> 1.0.4"
   s.add_development_dependency "flexmock", "~> 0.9.0"
+
+  # For legacy custom formatter regression tests
+  s.add_development_dependency "fuubar",                 "1.3.2"
+  s.add_development_dependency "nyan-cat-formatter",     "0.5.2"
+  s.add_development_dependency "rspec-instafail",        "0.2.4"
+  s.add_development_dependency "rspec_spinner",          "2.0.0"
+  s.add_development_dependency "rspec-extra-formatters", "1.0.0"
 end

spec/rspec/core/formatters/legacy_formatter_spec.rb

@@ -6,6 +6,19 @@
 RSpec.describe RSpec::Core::Formatters::LegacyFormatter do
   include FormatterSupport
 
+  it 'can access attributes provided by base class accessors in #initialize' do
+    klass = Class.new(LegacyFormatterUsingSubClassing) do
+      def initialize(*args)
+        example_count
+        super
+      end
+    end
+
+    config.add_formatter klass
+    expect(config.formatters.first).to be_a(RSpec::Core::Formatters::LegacyFormatter)
+    expect(config.formatters.first.formatter).to be_a(klass)
+  end
+
   [OldStyleFormatterExample, LegacyFormatterUsingSubClassing].each do |klass|
 
     describe "#{klass}" do

spec/rspec/core/formatters_spec.rb

@@ -44,11 +44,27 @@ module RSpec::Core::Formatters
         expect(loader.formatters.first).to be_an_instance_of(RSpec::Core::Formatters::LegacyFormatter)
       end
 
-      it "issues a deprecation on legacy formatter use" do
+      context "when a legacy formatter is added" do
         formatter_class = Struct.new(:output)
-        expect_warn_deprecation_with_call_site(__FILE__, __LINE__ + 2,
-          /The #{formatter_class} formatter uses the deprecated formatter interface/)
-        loader.add formatter_class, output
+
+        it "issues a deprecation" do
+          expect_warn_deprecation_with_call_site(__FILE__, __LINE__ + 2,
+            /The #{formatter_class} formatter uses the deprecated formatter interface/)
+          loader.add formatter_class, output
+        end
+
+        it "does not mistakenly add in the progress formatter" do
+          # When we issue a deprecation warning it triggers `setup_defaults`,
+          # which adds the progress formatter if it thinks no formatter has been
+          # added yet.
+          allow(RSpec).to receive(:warn_deprecation) do
+            loader.setup_default(StringIO.new, StringIO.new)
+          end
+
+          loader.add formatter_class, output
+
+          expect(loader.formatters.grep(RSpec::Core::Formatters::ProgressFormatter)).to eq([])
+        end
       end
 
       it "finds a formatter by class fully qualified name" do