Add scenario hook errors to first and last steps (#417)

features/events.feature

@@ -93,3 +93,64 @@ Feature: suite events
       | AfterSuite     | 1 |
 
     And the suite should have failed
+
+
+  Scenario: should add scenario hook errors to steps
+    Given a feature "normal.feature" file:
+      """
+      Feature: scenario hook errors
+
+        Scenario: failing before and after scenario
+          Then adding step state to context
+          And passing step
+
+        Scenario: failing before scenario
+          Then adding step state to context
+          And passing step
+
+        Scenario: failing after scenario
+          Then adding step state to context
+          And passing step
+
+      """
+    When I run feature suite with formatter "pretty"
+
+    Then the suite should have failed
+    And the rendered output will be as follows:
+    """
+      Feature: scenario hook errors
+
+        Scenario: failing before and after scenario # normal.feature:3
+          Then adding step state to context         # suite_context_test.go:0 -> InitializeScenario.func12
+          after scenario hook failed: failed in after scenario hook, step error: before scenario hook failed: failed in before scenario hook
+          And passing step                          # suite_context_test.go:0 -> InitializeScenario.func2
+
+        Scenario: failing before scenario   # normal.feature:7
+          Then adding step state to context # suite_context_test.go:0 -> InitializeScenario.func12
+          before scenario hook failed: failed in before scenario hook
+          And passing step                  # suite_context_test.go:0 -> InitializeScenario.func2
+
+        Scenario: failing after scenario    # normal.feature:11
+          Then adding step state to context # suite_context_test.go:0 -> InitializeScenario.func12
+          And passing step                  # suite_context_test.go:0 -> InitializeScenario.func2
+          after scenario hook failed: failed in after scenario hook
+
+      --- Failed steps:
+
+        Scenario: failing before and after scenario # normal.feature:3
+          Then adding step state to context # normal.feature:4
+            Error: after scenario hook failed: failed in after scenario hook, step error: before scenario hook failed: failed in before scenario hook
+
+        Scenario: failing before scenario # normal.feature:7
+          Then adding step state to context # normal.feature:8
+            Error: before scenario hook failed: failed in before scenario hook
+
+        Scenario: failing after scenario # normal.feature:11
+          And passing step # normal.feature:13
+            Error: after scenario hook failed: failed in after scenario hook
+
+
+      3 scenarios (3 failed)
+      6 steps (1 passed, 3 failed, 2 skipped)
+      0s
+    """

run_test.go

@@ -414,11 +414,11 @@ func Test_AllFeaturesRun(t *testing.T) {
 ...................................................................... 140
 ...................................................................... 210
 ...................................................................... 280
-...................................                                    315
+........................................                               320
 
 
-82 scenarios (82 passed)
+83 scenarios (83 passed)
-315 steps (315 passed)
+320 steps (320 passed)
 0s
 `
 

suite.go

@@ -70,12 +70,14 @@ func (s *suite) matchStep(step *messages.PickleStep) *models.StepDefinition {
 	return def
 }
 
-func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, prevStepErr error) (rctx context.Context, err error) {
+func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, prevStepErr error, isFirst, isLast bool) (rctx context.Context, err error) {
 	var (
 		match *models.StepDefinition
 		sr    = models.PickleStepResult{Status: models.Undefined}
 	)
 
+	rctx = ctx
+
 	// user multistep definitions may panic
 	defer func() {
 		if e := recover(); e != nil {
@@ -87,20 +89,7 @@ func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, prevS
 
 		defer func() {
 			// run after step handlers
-			for _, f := range s.afterStepHandlers {
+			rctx, err = s.runAfterStepHooks(ctx, step, sr.Status, err)
-				hctx, herr := f(rctx, step, sr.Status, err)
-
-				// Adding hook error to resulting error without breaking hooks loop.
-				if herr != nil {
-					if err == nil {
-						err = herr
-					} else {
-						err = fmt.Errorf("%v: %w", herr, err)
-					}
-				}
-
-				rctx = hctx
-			}
 		}()
 
 		if prevStepErr != nil {
@@ -113,6 +102,11 @@ func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, prevS
 
 		sr = models.NewStepResult(pickle.Id, step.Id, match)
 
+		// Trigger after scenario on failing or last step to attach possible hook error to step.
+		if (err == nil && isLast) || err != nil {
+			rctx, err = s.runAfterScenarioHooks(rctx, pickle, err)
+		}
+
 		switch err {
 		case nil:
 			sr.Status = models.Passed
@@ -133,18 +127,26 @@ func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, prevS
 		}
 	}()
 
-	// run before step handlers
+	// run before scenario handlers
-	for _, f := range s.beforeStepHandlers {
+	if isFirst {
-		ctx, err = f(ctx, step)
+		ctx, err = s.runBeforeScenarioHooks(ctx, pickle)
-		if err != nil {
-			return ctx, err
-		}
 	}
 
 	match = s.matchStep(step)
 	s.storage.MustInsertStepDefintionMatch(step.AstNodeIds[0], match)
 	s.fmt.Defined(pickle, step, match.GetInternalStepDefinition())
 
+	// run before step handlers
+	ctx, err = s.runBeforeStepHooks(ctx, step, err)
+
+	if err != nil {
+		sr = models.NewStepResult(pickle.Id, step.Id, match)
+		sr.Status = models.Failed
+		s.storage.MustInsertPickleStepResult(sr)
+
+		return ctx, err
+	}
+
 	if ctx, undef, err := s.maybeUndefined(ctx, step.Text, step.Argument); err != nil {
 		return ctx, err
 	} else if len(undef) > 0 {
@@ -183,6 +185,118 @@ func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, prevS
 	return ctx, err
 }
 
+func (s *suite) runBeforeStepHooks(ctx context.Context, step *Step, err error) (context.Context, error) {
+	hooksFailed := false
+
+	for _, f := range s.beforeStepHandlers {
+		hctx, herr := f(ctx, step)
+		if herr != nil {
+			hooksFailed = true
+
+			if err == nil {
+				err = herr
+			} else {
+				err = fmt.Errorf("%v, %w", herr, err)
+			}
+		}
+
+		if hctx != nil {
+			ctx = hctx
+		}
+	}
+
+	if hooksFailed {
+		err = fmt.Errorf("before step hook failed: %w", err)
+	}
+
+	return ctx, err
+}
+
+func (s *suite) runAfterStepHooks(ctx context.Context, step *Step, status StepResultStatus, err error) (context.Context, error) {
+	for _, f := range s.afterStepHandlers {
+		hctx, herr := f(ctx, step, status, err)
+
+		// Adding hook error to resulting error without breaking hooks loop.
+		if herr != nil {
+			if err == nil {
+				err = herr
+			} else {
+				err = fmt.Errorf("%v, %w", herr, err)
+			}
+		}
+
+		if hctx != nil {
+			ctx = hctx
+		}
+	}
+
+	return ctx, err
+}
+
+func (s *suite) runBeforeScenarioHooks(ctx context.Context, pickle *messages.Pickle) (context.Context, error) {
+	var err error
+
+	// run before scenario handlers
+	for _, f := range s.beforeScenarioHandlers {
+		hctx, herr := f(ctx, pickle)
+		if herr != nil {
+			if err == nil {
+				err = herr
+			} else {
+				err = fmt.Errorf("%v, %w", herr, err)
+			}
+		}
+
+		if hctx != nil {
+			ctx = hctx
+		}
+	}
+
+	if err != nil {
+		err = fmt.Errorf("before scenario hook failed: %w", err)
+	}
+
+	return ctx, err
+}
+
+func (s *suite) runAfterScenarioHooks(ctx context.Context, pickle *messages.Pickle, lastStepErr error) (context.Context, error) {
+	err := lastStepErr
+
+	hooksFailed := false
+	isStepErr := true
+
+	// run after scenario handlers
+	for _, f := range s.afterScenarioHandlers {
+		hctx, herr := f(ctx, pickle, err)
+
+		// Adding hook error to resulting error without breaking hooks loop.
+		if herr != nil {
+			hooksFailed = true
+
+			if err == nil {
+				isStepErr = false
+				err = herr
+			} else {
+				if isStepErr {
+					err = fmt.Errorf("step error: %w", err)
+					isStepErr = false
+				}
+				err = fmt.Errorf("%v, %w", herr, err)
+			}
+		}
+
+		if hctx != nil {
+			ctx = hctx
+		}
+	}
+
+	if hooksFailed {
+		err = fmt.Errorf("after scenario hook failed: %w", err)
+	}
+
+	return ctx, err
+}
+
 func (s *suite) maybeUndefined(ctx context.Context, text string, arg interface{}) (context.Context, []string, error) {
 	step := s.matchStepText(text)
 	if nil == step {
@@ -271,8 +385,10 @@ func (s *suite) runSteps(ctx context.Context, pickle *Scenario, steps []*Step) (
 		stepErr, err error
 	)
 
-	for _, step := range steps {
+	for i, step := range steps {
-		ctx, stepErr = s.runStep(ctx, pickle, step, err)
+		isLast := i == len(steps)-1
+		isFirst := i == 0
+		ctx, stepErr = s.runStep(ctx, pickle, step, err, isFirst, isLast)
 		switch stepErr {
 		case ErrUndefined:
 			// do not overwrite failed error
@@ -326,13 +442,8 @@ func (s *suite) runPickle(pickle *messages.Pickle) (err error) {
 		return ErrUndefined
 	}
 
-	// run before scenario handlers
+	// Before scenario hooks are aclled in context of first evaluated step
-	for _, f := range s.beforeScenarioHandlers {
+	// so that error from handler can be added to step.
-		ctx, err = f(ctx, pickle)
-		if err != nil {
-			return err
-		}
-	}
 
 	pr := models.PickleResult{PickleID: pickle.Id, StartedAt: utils.TimeNowFunc()}
 	s.storage.MustInsertPickleResult(pr)
@@ -342,21 +453,8 @@ func (s *suite) runPickle(pickle *messages.Pickle) (err error) {
 	// scenario
 	ctx, err = s.runSteps(ctx, pickle, pickle.Steps)
 
-	// run after scenario handlers
+	// After scenario handlers are called in context of last evaluated step
-	for _, f := range s.afterScenarioHandlers {
+	// so that error from handler can be added to step.
-		hctx, herr := f(ctx, pickle, err)
-
-		// Adding hook error to resulting error without breaking hooks loop.
-		if herr != nil {
-			if err == nil {
-				err = herr
-			} else {
-				err = fmt.Errorf("%v: %w", herr, err)
-			}
-		}
-
-		ctx = hctx
-	}
 
 	return err
 }

suite_context_test.go

@@ -426,6 +426,22 @@ func (tc *godogFeaturesScenario) iAmListeningToSuiteEvents() error {
 		return context.WithValue(ctx, ctxKey("BeforeScenario"), true), nil
 	})
 
+	scenarioContext.Before(func(ctx context.Context, sc *Scenario) (context.Context, error) {
+		if sc.Name == "failing before and after scenario" || sc.Name == "failing before scenario" {
+			return context.WithValue(ctx, ctxKey("AfterStep"), true), errors.New("failed in before scenario hook")
+		}
+
+		return ctx, nil
+	})
+
+	scenarioContext.After(func(ctx context.Context, sc *Scenario, err error) (context.Context, error) {
+		if sc.Name == "failing before and after scenario" || sc.Name == "failing after scenario" {
+			return ctx, errors.New("failed in after scenario hook")
+		}
+
+		return ctx, nil
+	})
+
 	scenarioContext.After(func(ctx context.Context, pickle *Scenario, err error) (context.Context, error) {
 		tc.events = append(tc.events, &firedEvent{"AfterScenario", []interface{}{pickle, err}})
 
@@ -678,14 +694,15 @@ func (tc *godogFeaturesScenario) theRenderOutputWillBe(docstring *DocString) err
 	expected = suiteCtxPtrReg.ReplaceAllString(expected, "*godogFeaturesScenario")
 
 	actual := tc.out.String()
-	actual = trimAllLines(actual)
 	actual = actualSuiteCtxReg.ReplaceAllString(actual, "suite_context_test.go:0")
 	actual = actualSuiteCtxFuncReg.ReplaceAllString(actual, "InitializeScenario.func$1")
+	actualTrimmed := actual
+	actual = trimAllLines(actual)
 
 	expectedRows := strings.Split(expected, "\n")
 	actualRows := strings.Split(actual, "\n")
 
-	return assertExpectedAndActual(assert.ElementsMatch, expectedRows, actualRows)
+	return assertExpectedAndActual(assert.ElementsMatch, expectedRows, actualRows, actualTrimmed)
 }
 
 func (tc *godogFeaturesScenario) theRenderXMLWillBe(docstring *DocString) error {