fix(StopOnFailure): Wire up Options.StopOnFailure to stop the suite on first step and scenario failure

CHANGELOG.md

@@ -8,6 +8,10 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt
 
 ## Unreleased
 
+### Fixed
+- Propagate `Options.StopOnFailure` into the suite so remaining before-step hooks are skipped once one fails, and add BDD coverage confirming later scenarios are skipped after a failure - ([770](https://github.com/cucumber/godog/pull/770) - [kcross-ctoken](https://github.com/kcross-ctoken))
+- Scenarios skipped entirely by `Options.StopOnFailure` are now recorded and reported: they and their steps show up as `skipped` in the summary counts instead of vanishing from the output - ([770](https://github.com/cucumber/godog/pull/770) - [kcross-ctoken](https://github.com/kcross-ctoken))
+
 ## [v0.16.0]
 
 ### Changed

features/events.feature

@@ -1,3 +1,4 @@
+@events
 Feature: suite events
   In order to run tasks before and after important events
   As a test suite
@@ -94,6 +95,60 @@ Feature: suite events
 
     And the suite should have failed
 
+  @StopOnFailure
+  Scenario: On Options.StopOnFailure Exit quickly
+    Given suite option StopOnFailure=true
+    And a feature "normal.feature" file:
+      """
+      Feature: On Options.StopOnFailure Exit quickly
+
+        Scenario: passing
+          Then adding step state to context
+          And passing step
+          And passing step
+
+        Scenario: failing mid step scenario
+          Then passing step
+          And failing step
+          And passing step
+
+        Scenario: never runs
+          Then passing step
+          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: On Options.StopOnFailure Exit quickly
+
+Scenario: passing                   # normal.feature:3
+Then adding step state to context # <autogenerated>:0 -> InitializeScenario.func17
+And passing step                  # <autogenerated>:0 -> InitializeScenario.func2
+And passing step                  # <autogenerated>:0 -> InitializeScenario.func2
+
+Scenario: failing mid step scenario # normal.feature:8
+Then passing step                 # <autogenerated>:0 -> InitializeScenario.func2
+And failing step                  # <autogenerated>:0 -> *godogFeaturesScenario
+intentional failure
+And passing step                  # <autogenerated>:0 -> InitializeScenario.func2
+
+Scenario: never runs # normal.feature:13
+Then passing step  # <autogenerated>:0 -> InitializeScenario.func2
+And passing step   # <autogenerated>:0 -> InitializeScenario.func2
+
+--- Failed steps:
+
+Scenario: failing mid step scenario # normal.feature:8
+And failing step # normal.feature:10
+Error: intentional failure
+
+
+3 scenarios (1 passed, 1 failed, 1 skipped)
+8 steps (4 passed, 1 failed, 3 skipped)
+0s
+    """
 
   Scenario: should add scenario hook errors to steps
     Given a feature "normal.feature" file:
@@ -153,4 +208,4 @@ Feature: suite events
       3 scenarios (3 failed)
       6 steps (1 passed, 3 failed, 2 skipped)
       0s
-    """
+    """

internal/formatters/fmt_base.go

@@ -91,7 +91,7 @@ func (f *Base) Ambiguous(*messages.Pickle, *messages.PickleStep, *formatters.Ste
 
 // Summary renders summary information.
 func (f *Base) Summary() {
-	var totalSc, passedSc, undefinedSc int
+	var totalSc, passedSc, undefinedSc, skippedSc int
 	var totalSt, passedSt, failedSt, skippedSt, pendingSt, undefinedSt, ambiguousSt int
 
 	pickleResults := f.Storage.MustGetPickleResults()
@@ -105,30 +105,41 @@ func (f *Base) Summary() {
 			prStatus = undefined
 		}
 
+		// a scenario that was never run (e.g. skipped entirely because of
+		// Options.StopOnFailure) has only skipped steps and no other status
+		ranAnyStep := false
+
 		for _, sr := range pickleStepResults {
 			totalSt++
 
 			switch sr.Status {
 			case passed:
 				passedSt++
+				ranAnyStep = true
 			case failed:
 				prStatus = failed
 				failedSt++
+				ranAnyStep = true
 			case ambiguous:
 				prStatus = ambiguous
 				ambiguousSt++
+				ranAnyStep = true
 			case skipped:
 				skippedSt++
 			case undefined:
 				prStatus = undefined
 				undefinedSt++
+				ranAnyStep = true
 			case pending:
 				prStatus = pending
 				pendingSt++
+				ranAnyStep = true
 			}
 		}
 
-		if prStatus == passed {
+		if len(pickleStepResults) > 0 && !ranAnyStep {
+			skippedSc++
+		} else if prStatus == passed {
 			passedSc++
 		} else if prStatus == undefined {
 			undefinedSc++
@@ -165,6 +176,9 @@ func (f *Base) Summary() {
 		scenarios = append(scenarios, green(fmt.Sprintf("%d passed", passedSc)))
 	}
 	scenarios = append(scenarios, parts...)
+	if skippedSc > 0 {
+		scenarios = append(scenarios, cyan(fmt.Sprintf("%d skipped", skippedSc)))
+	}
 
 	testRunStartedAt := f.Storage.MustGetTestRunStarted().StartedAt
 	elapsed := utils.TimeNowFunc().Sub(testRunStartedAt)

internal/formatters/fmt_junit.go

@@ -163,6 +163,9 @@ func (f *JUnit) buildJUNITPackageSuite() JunitPackageSuite {
 						Message: fmt.Sprintf("Step %s", pickleStep.Text),
 					})
 				case skipped:
+					if tc.Status == "" {
+						tc.Status = skipped.String()
+					}
 					tc.Error = append(tc.Error, &junitError{
 						Type:    "skipped",
 						Message: fmt.Sprintf("Step %s", pickleStep.Text),

run.go

@@ -69,6 +69,7 @@ func (r *runner) concurrent(rate int) (failed bool) {
 			storage:        r.storage,
 			defaultContext: r.defaultContext,
 			testingT:       r.testingT,
+			stopOnFailure:  r.stopOnFailure,
 		},
 	}
 	if r.testSuiteInitializer != nil {
@@ -111,10 +112,6 @@ func (r *runner) concurrent(rate int) (failed bool) {
 					<-queue // free a space in queue
 				}()
 
-				if r.stopOnFailure && *fail {
-					return
-				}
-
 				// Copy base suite.
 				suite := *testSuiteContext.suite
 				if rate > 1 {
@@ -130,6 +127,11 @@ func (r *runner) concurrent(rate int) (failed bool) {
 					r.scenarioInitializer(&sc)
 				}
 
+				if r.stopOnFailure && *fail {
+					suite.skipPickle(pickle)
+					return
+				}
+
 				err := suite.runPickle(pickle)
 				if suite.shouldFail(err) {
 					copyLock.Lock()

run_test.go

@@ -380,18 +380,10 @@ func Test_RandomizeRun_WithStaticSeed(t *testing.T) {
 		ctx.Step(`^odd (\d+) and even (\d+) number$`, oddEvenStepDef)
 	}
 
-	expectedStatus, expectedOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter, noConcurrencyFlag,
-		noRandomFlag, []string{featurePath},
-	)
+	expectedStatus, expectedOutput := testRun(t, fmtOutputScenarioInitializer, formatter, noConcurrencyFlag, noRandomFlag, []string{featurePath}, "")
 
 	const staticSeed int64 = 1
-	actualStatus, actualOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter, noConcurrencyFlag,
-		staticSeed, []string{featurePath},
-	)
+	actualStatus, actualOutput := testRun(t, fmtOutputScenarioInitializer, formatter, noConcurrencyFlag, staticSeed, []string{featurePath}, "")
 
 	actualSeed := parseSeed(actualOutput)
 	assert.Equal(t, staticSeed, actualSeed)
@@ -419,20 +411,12 @@ func Test_RandomizeRun_RerunWithSeed(t *testing.T) {
 		ctx.Step(`^odd (\d+) and even (\d+) number$`, oddEvenStepDef)
 	}
 
-	expectedStatus, expectedOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter, noConcurrencyFlag,
-		createRandomSeedFlag, []string{featurePath},
-	)
+	expectedStatus, expectedOutput := testRun(t, fmtOutputScenarioInitializer, formatter, noConcurrencyFlag, createRandomSeedFlag, []string{featurePath}, "")
 
 	expectedSeed := parseSeed(expectedOutput)
 	assert.NotZero(t, expectedSeed)
 
-	actualStatus, actualOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter, noConcurrencyFlag,
-		expectedSeed, []string{featurePath},
-	)
+	actualStatus, actualOutput := testRun(t, fmtOutputScenarioInitializer, formatter, noConcurrencyFlag, expectedSeed, []string{featurePath}, "")
 
 	actualSeed := parseSeed(actualOutput)
 
@@ -454,11 +438,7 @@ func Test_FormatOutputRun(t *testing.T) {
 		ctx.Step(`^odd (\d+) and even (\d+) number$`, oddEvenStepDef)
 	}
 
-	expectedStatus, expectedOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter, noConcurrencyFlag,
-		noRandomFlag, []string{featurePath},
-	)
+	expectedStatus, expectedOutput := testRun(t, fmtOutputScenarioInitializer, formatter, noConcurrencyFlag, noRandomFlag, []string{featurePath}, "")
 
 	dir := filepath.Join(os.TempDir(), t.Name())
 	err := os.MkdirAll(dir, 0755)
@@ -468,11 +448,7 @@ func Test_FormatOutputRun(t *testing.T) {
 
 	file := filepath.Join(dir, "result.xml")
 
-	actualStatus, actualOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter+":"+file, noConcurrencyFlag,
-		noRandomFlag, []string{featurePath},
-	)
+	actualStatus, actualOutput := testRun(t, fmtOutputScenarioInitializer, formatter+":"+file, noConcurrencyFlag, noRandomFlag, []string{featurePath}, "")
 
 	result, err := ioutil.ReadFile(file)
 	require.NoError(t, err)
@@ -502,11 +478,7 @@ func Test_FormatOutputRun_Error(t *testing.T) {
 	file := filepath.Join(dir, "result.xml")
 
 	// next test is expected to log: couldn't create file with name: )
-	actualStatus, actualOutput := testRun(t,
-		fmtOutputScenarioInitializer,
-		formatter+":"+file, noConcurrencyFlag,
-		noRandomFlag, []string{featurePath},
-	)
+	actualStatus, actualOutput := testRun(t, fmtOutputScenarioInitializer, formatter+":"+file, noConcurrencyFlag, noRandomFlag, []string{featurePath}, "")
 
 	assert.Equal(t, expectedStatus, actualStatus)
 	assert.Equal(t, expectedOutput, actualOutput)
@@ -515,6 +487,8 @@ func Test_FormatOutputRun_Error(t *testing.T) {
 	assert.Error(t, err)
 }
 
+// The expected string here is a bad way of testing the output because it changes with each feature added.
+// This means we have to alter 3 different places to add one feature adding drag to maintenance.
 func Test_AllFeaturesRun(t *testing.T) {
 	const concurrency = 100
 	const noRandomFlag = 0
@@ -526,19 +500,15 @@ func Test_AllFeaturesRun(t *testing.T) {
 ...................................................................... 280
 ...................................................................... 350
 ...................................................................... 420
-...                                                                    423
+.........                                                              429
 
 
-108 scenarios (108 passed)
-423 steps (423 passed)
+109 scenarios (109 passed)
+429 steps (429 passed)
 0s
 `
 
-	actualStatus, actualOutput := testRun(t,
-		InitializeScenario,
-		format, concurrency,
-		noRandomFlag, []string{"features"},
-	)
+	actualStatus, actualOutput := testRun(t, InitializeScenario, format, concurrency, noRandomFlag, []string{"features"}, "")
 
 	assert.Equal(t, exitSuccess, actualStatus)
 	assert.Equal(t, expected, actualOutput)
@@ -555,11 +525,11 @@ func Test_AllFeaturesRunAsSubtests(t *testing.T) {
 ...................................................................... 280
 ...................................................................... 350
 ...................................................................... 420
-...                                                                    423
+.........                                                              429
 
 
-108 scenarios (108 passed)
-423 steps (423 passed)
+109 scenarios (109 passed)
+429 steps (429 passed)
 0s
 `
 
@@ -605,16 +575,8 @@ func Test_FormatterConcurrencyRun(t *testing.T) {
 		t.Run(
 			fmt.Sprintf("%s/concurrency/%d", formatter, concurrency),
 			func(t *testing.T) {
-				expectedStatus, expectedOutput := testRun(t,
-					fmtOutputScenarioInitializer,
-					formatter, noConcurrency,
-					noRandomFlag, featurePaths,
-				)
-				actualStatus, actualOutput := testRun(t,
-					fmtOutputScenarioInitializer,
-					formatter, concurrency,
-					noRandomFlag, featurePaths,
-				)
+				expectedStatus, expectedOutput := testRun(t, fmtOutputScenarioInitializer, formatter, noConcurrency, noRandomFlag, featurePaths, "")
+				actualStatus, actualOutput := testRun(t, fmtOutputScenarioInitializer, formatter, concurrency, noRandomFlag, featurePaths, "")
 
 				assert.Equal(t, expectedStatus, actualStatus)
 				assertOutput(t, formatter, expectedOutput, actualOutput)
@@ -623,21 +585,21 @@ func Test_FormatterConcurrencyRun(t *testing.T) {
 	}
 }
 
-func testRun(
-	t *testing.T,
+func testRun(t *testing.T,
 	scenarioInitializer func(*ScenarioContext),
 	format string,
 	concurrency int,
 	randomSeed int64,
 	featurePaths []string,
-) (int, string) {
+	tags string) (int, string) {
 	t.Helper()
 
 	opts := Options{
 		Format:      format,
 		Paths:       featurePaths,
 		Concurrency: concurrency,
 		Randomize:   randomSeed,
+		Tags:        tags,
 	}
 
 	return testRunWithOptions(t, opts, scenarioInitializer)

suite.go

@@ -311,6 +311,9 @@ func (s *suite) runBeforeStepHooks(ctx context.Context, step *Step, err error) (
 		if hctx != nil {
 			ctx = hctx
 		}
+		if s.stopOnFailure && err != nil {
+			break
+		}
 	}
 
 	if hooksFailed {
@@ -650,6 +653,27 @@ func (s *suite) runPickle(pickle *messages.Pickle) (err error) {
 	return err
 }
 
+// skipPickle records a pickle and all of its steps as skipped, without
+// executing anything. Used when Options.StopOnFailure causes a scenario
+// to be skipped entirely because an earlier scenario has already failed.
+func (s *suite) skipPickle(pickle *messages.Pickle) {
+	pr := models.PickleResult{PickleID: pickle.Id, StartedAt: utils.TimeNowFunc()}
+	s.storage.MustInsertPickleResult(pr)
+
+	s.fmt.Pickle(pickle)
+
+	for _, step := range pickle.Steps {
+		match, _ := s.matchStep(step)
+
+		s.storage.MustInsertStepDefintionMatch(step.AstNodeIds[0], match)
+		s.fmt.Defined(pickle, step, match.GetInternalStepDefinition())
+
+		sr := models.NewStepResult(models.Skipped, pickle.Id, step.Id, match, nil, nil)
+		s.storage.MustInsertPickleStepResult(sr)
+		s.fmt.Skipped(pickle, step, match.GetInternalStepDefinition())
+	}
+}
+
 type joinedError struct {
 	err1 error
 	err2 error