Improve handling of additional around/after exceptions.

Now that we have nice formatting logic for multiple
exception errors, let’s leverage that to improve the
output for this case.

Fixes #1966.

Author: myronmarston

features/expectation_framework_integration/aggregating_failures.feature

@@ -13,7 +13,7 @@ Feature: Aggregating Failures
       Response = Struct.new(:status, :headers, :body)
 
       class Client
-        def self.make_request
+        def self.make_request(url='/')
           Response.new(404, { "Content-Type" => "text/plain" }, "Not Found")
         end
       end
@@ -25,6 +25,17 @@ Feature: Aggregating Failures
       require 'client'
 
       RSpec.describe Client do
+        after do
+          # this should be appended to failure list
+          expect(false).to be(true), "after hook failure"
+        end
+
+        around do |ex|
+          ex.run
+          # this should also be appended to failure list
+          expect(false).to be(true), "around hook failure"
+        end
+
         it "returns a successful response" do
           response = Client.make_request
 
@@ -42,32 +53,44 @@ Feature: Aggregating Failures
       Failures:
 
         1) Client returns a successful response
-           Got 3 failures from failure aggregation block "testing reponse".
-           # ./spec/use_block_form_spec.rb:7
+           Got 3 failures:
 
-           1.1) Failure/Error: expect(response.status).to eq(200)
+           1.1) Got 3 failures from failure aggregation block "testing reponse".
+                # ./spec/use_block_form_spec.rb:18:in `block (2 levels) in <top (required)>'
+                # ./spec/use_block_form_spec.rb:10:in `block (2 levels) in <top (required)>'
 
-                  expected: 200
-                       got: 404
+                1.1.1) Failure/Error: expect(response.status).to eq(200)
 
-                  (compared using ==)
-                # ./spec/use_block_form_spec.rb:8
+                         expected: 200
+                              got: 404
 
-           1.2) Failure/Error: expect(response.headers).to include("Content-Type" => "application/json")
-                  expected {"Content-Type" => "text/plain"} to include {"Content-Type" => "application/json"}
-                  Diff:
-                  @@ -1,2 +1,2 @@
-                  -[{"Content-Type"=>"application/json"}]
-                  +"Content-Type" => "text/plain",
-                # ./spec/use_block_form_spec.rb:9
+                         (compared using ==)
+                       # ./spec/use_block_form_spec.rb:19:in `block (3 levels) in <top (required)>'
 
-           1.3) Failure/Error: expect(response.body).to eq('{"message":"Success"}')
+                1.1.2) Failure/Error: expect(response.headers).to include("Content-Type" => "application/json")
+                         expected {"Content-Type" => "text/plain"} to include {"Content-Type" => "application/json"}
+                         Diff:
+                         @@ -1,2 +1,2 @@
+                         -[{"Content-Type"=>"application/json"}]
+                         +"Content-Type" => "text/plain",
+                       # ./spec/use_block_form_spec.rb:20:in `block (3 levels) in <top (required)>'
 
-                  expected: "{\"message\":\"Success\"}"
-                       got: "Not Found"
+                1.1.3) Failure/Error: expect(response.body).to eq('{"message":"Success"}')
 
-                  (compared using ==)
-                # ./spec/use_block_form_spec.rb:10
+                         expected: "{\"message\":\"Success\"}"
+                              got: "Not Found"
+
+                         (compared using ==)
+                       # ./spec/use_block_form_spec.rb:21:in `block (3 levels) in <top (required)>'
+
+           1.2) Failure/Error: expect(false).to be(true), "after hook failure"
+                  after hook failure
+                # ./spec/use_block_form_spec.rb:6:in `block (2 levels) in <top (required)>'
+                # ./spec/use_block_form_spec.rb:10:in `block (2 levels) in <top (required)>'
+
+           1.3) Failure/Error: expect(false).to be(true), "around hook failure"
+                  around hook failure
+                # ./spec/use_block_form_spec.rb:12:in `block (2 levels) in <top (required)>'
       """
 
   Scenario: Use `:aggregate_failures` metadata

lib/rspec/core/example.rb

@@ -306,24 +306,17 @@ def inspect
       #
       # Used internally to set an exception in an after hook, which
       # captures the exception but doesn't raise it.
-      def set_exception(exception, context=nil)
+      def set_exception(exception)
         if pending? && !(Pending::PendingExampleFixedError === exception)
           execution_result.pending_exception = exception
-        else
-          if @exception
-            # An error has already been set; we don't want to override it,
-            # but we also don't want silence the error, so let's print it.
-            msg = <<-EOS
-
-  An error occurred #{context}
-    #{exception.class}: #{exception.message}
-    occurred at #{exception.backtrace.first}
-
-            EOS
-            RSpec.configuration.reporter.message(msg)
+        elsif @exception
+          unless RSpec::Core::MultipleExceptionError === @exception
+            @exception = RSpec::Core::MultipleExceptionError.new(@exception)
           end
 
-          @exception ||= exception
+          @exception.add exception
+        else
+          @exception = exception
         end
       end
 
@@ -348,10 +341,10 @@ def skip_with_exception(reporter, exception)
       end
 
       # @private
-      def instance_exec_with_rescue(context, &block)
+      def instance_exec_with_rescue(&block)
         @example_group_instance.instance_exec(self, &block)
       rescue Exception => e
-        set_exception(e, context)
+        set_exception(e)
       end
 
       # @private
@@ -368,7 +361,7 @@ def hooks
       def with_around_example_hooks
         hooks.run(:around, :example, self) { yield }
       rescue Exception => e
-        set_exception(e, "in an `around(:example)` hook")
+        set_exception(e)
       end
 
       def start(reporter)
@@ -548,7 +541,7 @@ def initialize
       end
 
       # To ensure we don't silence errors.
-      def set_exception(exception, _context=nil)
+      def set_exception(exception)
         raise exception
       end
     end

lib/rspec/core/hooks.rb

@@ -361,7 +361,7 @@ def run(example)
       # @private
       class AfterHook < Hook
         def run(example)
-          example.instance_exec_with_rescue("in an after hook", &block)
+          example.instance_exec_with_rescue(&block)
         end
       end
 

spec/rspec/core/example_spec.rb

@@ -31,11 +31,8 @@ def metadata_hash(*args)
   end
 
   describe "#exception" do
-    it "supplies the first exception raised, if any" do
-      RSpec.configuration.output_stream = StringIO.new
-
+    it "supplies the exception raised, if there is one" do
       example = example_group.example { raise "first" }
-      example_group.after { raise "second" }
       example_group.run
       expect(example.exception.message).to eq("first")
     end
@@ -45,6 +42,25 @@ def metadata_hash(*args)
       example_group.run
       expect(example.exception).to be_nil
     end
+
+    it 'provides a `MultipleExceptionError` if there are multiple exceptions (e.g. from `it`, `around` and `after`)' do
+      the_example = nil
+
+      after_ex   = StandardError.new("after")
+      around_ex  = StandardError.new("around")
+      example_ex = StandardError.new("example")
+
+      RSpec.describe do
+        the_example = example { raise example_ex }
+        after { raise after_ex }
+        around { |ex| ex.run; raise around_ex }
+      end.run
+
+      expect(the_example.exception).to have_attributes(
+        :class => RSpec::Core::MultipleExceptionError,
+        :all_exceptions => [example_ex, after_ex, around_ex]
+      )
+    end
   end
 
   describe "when there is an explicit description" do
@@ -401,65 +417,20 @@ def expect_gc(opts)
       end
     end
 
-    context 'when the example raises an error' do
-      def run_and_capture_reported_message(group)
-        reported_msg = nil
-        # We can't use should_receive(:message).with(/.../) here,
-        # because if that fails, it would fail within our example-under-test,
-        # and since there's already two errors, it would just be reported again.
-        allow(RSpec.configuration.reporter).to receive(:message) { |msg| reported_msg = msg }
-        group.run
-        reported_msg
-      end
+    it "leaves raised exceptions unmodified (GH-1103)" do
+      # set the backtrace, otherwise MRI will build a whole new object,
+      # and thus mess with our expectations. Rubinius and JRuby are not
+      # affected.
+      exception = StandardError.new
+      exception.set_backtrace([])
 
-      it "prints any around hook errors rather than silencing them" do
-        group = RSpec.describe do
-          around(:each) { |e| e.run; raise "around" }
-          example("e") { raise "example" }
-        end
-
-        message = run_and_capture_reported_message(group)
-        expect(message).to match(/An error occurred in an `around.* hook/i)
-      end
-
-      it "prints any after hook errors rather than silencing them" do
-        group = RSpec.describe do
-          after(:each) { raise "after" }
-          example("e") { raise "example" }
-        end
-
-        message = run_and_capture_reported_message(group)
-        expect(message).to match(/An error occurred in an after.* hook/i)
-      end
-
-      it "does not print mock expectation errors" do
-        group = RSpec.describe do
-          example do
-            foo = double
-            expect(foo).to receive(:bar)
-            raise "boom"
-          end
-        end
-
-        message = run_and_capture_reported_message(group)
-        expect(message).to be_nil
+      group = RSpec.describe do
+        example { raise exception.freeze }
       end
+      group.run
 
-      it "leaves a raised exception unmodified (GH-1103)" do
-        # set the backtrace, otherwise MRI will build a whole new object,
-        # and thus mess with our expectations. Rubinius and JRuby are not
-        # affected.
-        exception = StandardError.new
-        exception.set_backtrace([])
-
-        group = RSpec.describe do
-          example { raise exception.freeze }
-        end
-        group.run
-
-        actual = group.examples.first.execution_result.exception
-        expect(actual.__id__).to eq(exception.__id__)
-      end
+      actual = group.examples.first.execution_result.exception
+      expect(actual.__id__).to eq(exception.__id__)
     end
 
     context "with --dry-run" do
@@ -752,6 +723,21 @@ def expect_pending_result(example)
       expect(ex).to fail_with(RSpec::Mocks::MockExpectationError)
     end
 
+    it 'skips mock verification if the example has already failed' do
+      ex = nil
+      boom = StandardError.new("boom")
+
+      RSpec.describe do
+        ex = example do
+          dbl = double
+          expect(dbl).to receive(:Foo)
+          raise boom
+        end
+      end.run
+
+      expect(ex.exception).to be boom
+    end
+
     it 'allows `after(:example)` hooks to satisfy mock expectations, since examples are not complete until their `after` hooks run' do
       ex = nil