Around hooks not executing in correct world context

features/docs/exception_in_around_hook.feature

@@ -0,0 +1,80 @@
+Feature: Exceptions in Around Hooks
+
+  Around hooks are awkward beasts to handle internally.
+
+  Right now, if there's an error in your Around hook before you call `block.call`,
+  we won't even print the steps for the scenario.
+
+  This is because that `block.call` invokes all the logic that would tell Cucumber's
+  UI about the steps in your scenario. If we never reach that code, we'll never be
+  told about them.
+
+  There's another scenario to consider, where the exception occurs after the steps
+  have been run. How would we want to report in that case?
+
+  Scenario: Exception before the test case is run
+    Given the standard step definitions
+    And a file named "features/support/env.rb" with:
+      """
+      Around do |scenario, block|
+        fail "this should be reported"
+        block.call
+      end
+      """
+    And a file named "features/test.feature" with:
+      """
+      Feature:
+        Scenario:
+          Given this step passes
+      """
+    When I run `cucumber -q`
+    Then it should fail with exactly:
+      """
+      Feature: 
+
+        Scenario: 
+        this should be reported (RuntimeError)
+        ./features/support/env.rb:2:in `Around'
+
+      Failing Scenarios:
+      cucumber features/test.feature:2
+
+      1 scenario (1 failed)
+      0 steps
+      0m0.012s
+
+      """
+
+  Scenario: Exception after the test case is run
+    Given the standard step definitions
+    And a file named "features/support/env.rb" with:
+      """
+      Around do |scenario, block|
+        block.call
+        fail "this should be reported"
+      end
+      """
+    And a file named "features/test.feature" with:
+      """
+      Feature:
+        Scenario:
+          Given this step passes
+      """
+    When I run `cucumber -q`
+    Then it should fail with exactly:
+      """
+      Feature: 
+
+        Scenario: 
+          Given this step passes
+            this should be reported (RuntimeError)
+            ./features/support/env.rb:3:in `Around'
+
+      Failing Scenarios:
+      cucumber features/test.feature:2
+
+      1 scenario (1 failed)
+      1 step (1 passed)
+      0m0.012s
+
+      """

features/docs/wire_protocol/handle_unexpected_response.feature

@@ -24,7 +24,7 @@ Feature: Handle unexpected response
       | ["begin_scenario"]                                   | ["yikes"]                           |
       | ["step_matches",{"name_to_match":"we're all wired"}] | ["success",[{"id":"1", "args":[]}]] |
     When I run `cucumber -f pretty`
-    Then the stdout should contain:
+    Then the output should contain:
       """
       undefined method `handle_yikes'
       """

features/docs/writing_support_code/around_hooks.feature

@@ -227,3 +227,34 @@ Feature: Around hooks
       2 steps (2 passed)
 
       """
+
+  Scenario: Around Hooks and the Custom World
+    Given a file named "features/step_definitions/steps.rb" with:
+      """
+      Then /^the world should be available in the hook$/ do
+        $previous_world = self
+        expect($hook_world).to eq(self)
+      end
+
+      Then /^what$/ do
+        expect($hook_world).not_to eq($previous_world)
+      end
+      """
+    And a file named "features/support/hooks.rb" with:
+      """
+      Around do |scenario, block|
+        $hook_world = self
+        block.call
+      end
+      """
+    And a file named "features/f.feature" with:
+      """
+      Feature: Around hooks
+        Scenario: using hook
+          Then the world should be available in the hook
+
+        Scenario: using the same hook
+          Then what
+      """
+    When I run `cucumber features/f.feature`
+    Then it should pass

lib/cucumber/filters/prepare_world.rb

@@ -17,11 +17,18 @@ def initialize(runtime, original_test_case)
         end
 
         def test_case
-          init_scenario = Cucumber::Hooks.before_hook(@original_test_case.source) do
+          init_scenario = Cucumber::Hooks.around_hook(@original_test_case.source) do |continue|
             @runtime.begin_scenario(scenario)
+            continue.call
           end
-          steps = [init_scenario] + @original_test_case.test_steps
-          @original_test_case.with_steps(steps)
+          around_hooks = [init_scenario] + @original_test_case.around_hooks
+
+          default_hook = Cucumber::Hooks.before_hook(@original_test_case.source) do
+            #no op - legacy format adapter expects a before hooks
+          end
+          steps = [default_hook] + @original_test_case.test_steps
+
+          @original_test_case.with_around_hooks(around_hooks).with_steps(steps)
         end
 
         private

lib/cucumber/formatter/legacy_api/adapter.rb

@@ -126,6 +126,30 @@ def timer
           end
         end
 
+        module TestCaseSource
+          def self.for(test_case, result)
+            collector = Collector.new
+            test_case.describe_source_to collector, result
+            collector.result.freeze
+          end
+
+          class Collector
+            attr_reader :result
+
+            def initialize
+              @result = CaseSource.new
+            end
+
+            def method_missing(name, node, test_case_result, *args)
+              result.send "#{name}=", node
+            end
+          end
+
+          require 'ostruct'
+          class CaseSource < OpenStruct
+          end
+        end
+
         module TestStepSource
           def self.for(test_step, result)
             collector = Collector.new
@@ -194,6 +218,7 @@ def before
 
           def before_test_case(test_case)
             @before_hook_results = Ast::HookResultCollection.new
+            @test_step_results = []
           end
 
           def before_test_step(test_step)
@@ -204,13 +229,20 @@ def after_test_step(test_step, result)
             # TODO: stop calling self, and describe source to another object
             test_step.describe_source_to(self, result)
             print_step
+            @test_step_results << result
           end
 
-          def after_test_case(*args)
+          def after_test_case(test_case, test_case_result)
             if current_test_step_source && current_test_step_source.step_result.nil?
               switch_step_container
             end
 
+            if test_case_result.failed? && !any_test_steps_failed?
+              # around hook must have failed. Print the error.
+              switch_step_container(TestCaseSource.for(test_case, test_case_result))
+              LegacyResultBuilder.new(test_case_result).describe_exception_to formatter
+            end
+
             # messages and embedding should already have been handled, but just in case...
             @delayed_messages.each { |message| formatter.puts(message) }
             @delayed_embeddings.each { |embedding| embedding.send_to_formatter(formatter) }
@@ -276,8 +308,12 @@ def after
           attr_reader :before_hook_results
           private :before_hook_results
 
-          def switch_step_container
-            switch_to_child select_step_container(current_test_step_source), current_test_step_source
+          def any_test_steps_failed?
+            @test_step_results.any? &:failed?
+          end
+
+          def switch_step_container(source = current_test_step_source)
+            switch_to_child select_step_container(source), source
           end
 
           def select_step_container(source)

spec/cucumber/formatter/legacy_api/adapter_spec.rb

@@ -1884,7 +1884,7 @@ def apply_after_hooks(test_case)
         end
 
         context 'with an exception in an after hook but no steps' do
-          it 'prints the exception after the steps' do
+          it 'prints the exception after the scenario name' do
             filters = [
               Filters::ActivateSteps.new(SimpleStepDefinitions.new),
               Filters::ApplyAfterHooks.new(FailingAfterHook.new),
@@ -1914,6 +1914,94 @@ def apply_after_hooks(test_case)
             ])
           end
         end
+
+        context 'with an exception in an around hook before the test case is run' do
+          class FailingAroundHookBeforeRunningTestCase
+            def find_around_hooks(test_case)
+              [
+                Hooks.around_hook(test_case.source) { raise Failure }
+              ]
+            end
+          end
+
+          it 'prints the exception after the scenario name' do
+            filters = [
+              Filters::ActivateSteps.new(SimpleStepDefinitions.new),
+              Filters::ApplyAroundHooks.new(FailingAroundHookBeforeRunningTestCase.new),
+              AddBeforeAndAfterHooks.new
+            ]
+            execute_gherkin(filters) do
+              feature do
+                scenario do
+                end
+              end
+            end
+
+            expect( formatter.legacy_messages ).to eq([
+              :before_features,
+                :before_feature,
+                  :before_tags,
+                  :after_tags,
+                  :feature_name,
+                  :before_feature_element,
+                    :before_tags,
+                    :after_tags,
+                    :scenario_name,
+                    :exception,
+                  :after_feature_element,
+                :after_feature,
+              :after_features,
+            ])
+          end
+        end
+
+        context 'with an exception in an around hook after the test case is run' do
+          class FailingAroundHookAfterRunningTestCase
+            def find_around_hooks(test_case)
+              [
+                Hooks.around_hook(test_case.source) { |run_test_case| run_test_case.call; raise Failure }
+              ]
+            end
+          end
+
+          it 'prints the exception after the scenario name' do
+            filters = [
+              Filters::ActivateSteps.new(SimpleStepDefinitions.new),
+              Filters::ApplyAroundHooks.new(FailingAroundHookAfterRunningTestCase.new),
+              AddBeforeAndAfterHooks.new
+            ]
+            execute_gherkin(filters) do
+              feature do
+                scenario do
+                  step
+                end
+              end
+            end
+
+            expect( formatter.legacy_messages ).to eq([
+              :before_features,
+                :before_feature,
+                  :before_tags,
+                  :after_tags,
+                  :feature_name,
+                  :before_feature_element,
+                    :before_tags,
+                    :after_tags,
+                    :scenario_name,
+                    :before_steps,
+                    :before_step,
+                    :before_step_result,
+                    :step_name,
+                    :after_step_result,
+                    :after_step,
+                    :exception,
+                    :after_steps,
+                  :after_feature_element,
+                :after_feature,
+              :after_features,
+            ])
+          end
+        end
       end
 
       it 'passes nil as the multiline arg when there is none' do

spec/cucumber/formatter/pretty_spec.rb

@@ -21,6 +21,19 @@ module Formatter
             run_defined_feature
           end
 
+          describe "with a scenario with no steps" do
+            define_feature <<-FEATURE
+          Feature: Banana party
+
+            Scenario: Monkey eats banana
+            FEATURE
+
+            it "outputs the scenario name" do
+              expect(@out.string).to include "Scenario: Monkey eats banana"
+            end
+          end
+
+
           describe "with a scenario" do
             define_feature <<-FEATURE
           Feature: Banana party