Add flaky test mitigation (#261)

* add flaky test mitigation

* Add integration test for flake flags

* Add flaky test args to BuildFlagArgs

* Remove requiring more than one pass

* Better documentation

* add concept of flake to spec and suite summary

config/config.go

@@ -31,6 +31,7 @@ type GinkgoConfigType struct {
 	SkipMeasurements   bool
 	FailOnPending      bool
 	FailFast           bool
+	FlakeAttempts      int
 	EmitSpecProgress   bool
 	DryRun             bool
 
@@ -75,6 +76,8 @@ func Flags(flagSet *flag.FlagSet, prefix string, includeParallelFlags bool) {
 
 	flagSet.BoolVar(&(GinkgoConfig.RegexScansFilePath), prefix+"regexScansFilePath", false, "If set, ginkgo regex matching also will look at the file path (code location).")
 
+	flagSet.IntVar(&(GinkgoConfig.FlakeAttempts), prefix+"flakeAttempts", 1, "Make up to this many attempts to run each spec. Please note that if any of the attempts succeed, the suite will not be failed. But any failures will still be recorded.")
+
 	flagSet.BoolVar(&(GinkgoConfig.EmitSpecProgress), prefix+"progress", false, "If set, ginkgo will emit progress information as each spec runs to the GinkgoWriter.")
 
 	if includeParallelFlags {
@@ -128,6 +131,10 @@ func BuildFlagArgs(prefix string, ginkgo GinkgoConfigType, reporter DefaultRepor
 		result = append(result, fmt.Sprintf("--%sskip=%s", prefix, ginkgo.SkipString))
 	}
 
+	if ginkgo.FlakeAttempts > 1 {
+		result = append(result, fmt.Sprintf("--%sflakeAttempts=%d", prefix, ginkgo.FlakeAttempts))
+	}
+
 	if ginkgo.EmitSpecProgress {
 		result = append(result, fmt.Sprintf("--%sprogress", prefix))
 	}

integration/_fixtures/flags_tests/flags_test.go

@@ -79,4 +79,12 @@ var _ = Describe("Testing various flags", func() {
 			Ω(true).Should(Equal(false))
 		})
 	})
+
+	Describe("a flaky test", func() {
+		runs := 0
+		It("should only pass the second time it's run", func() {
+			runs++
+			Ω(runs).Should(BeNumerically("==", 2))
+		})
+	})
 })

integration/flags_test.go

@@ -32,7 +32,7 @@ var _ = Describe("Flags Specs", func() {
 		Ω(output).Should(ContainSubstring("10 Passed"))
 		Ω(output).Should(ContainSubstring("0 Failed"))
 		Ω(output).Should(ContainSubstring("1 Pending"))
-		Ω(output).Should(ContainSubstring("2 Skipped"))
+		Ω(output).Should(ContainSubstring("3 Skipped"))
 		Ω(output).Should(ContainSubstring("[PENDING]"))
 		Ω(output).Should(ContainSubstring("marshmallow"))
 		Ω(output).Should(ContainSubstring("chocolate"))
@@ -82,11 +82,11 @@ var _ = Describe("Flags Specs", func() {
 		Ω(output).Should(ContainSubstring("3 Passed"))
 		Ω(output).Should(ContainSubstring("0 Failed"))
 		Ω(output).Should(ContainSubstring("0 Pending"))
-		Ω(output).Should(ContainSubstring("10 Skipped"))
+		Ω(output).Should(ContainSubstring("11 Skipped"))
 	})
 
 	It("should override the programmatic focus when told to skip", func() {
-		session := startGinkgo(pathToTest, "--noColor", "--skip=marshmallow|failing")
+		session := startGinkgo(pathToTest, "--noColor", "--skip=marshmallow|failing|flaky")
 		Eventually(session).Should(gexec.Exit(0))
 		output := string(session.Out.Contents())
 
@@ -96,7 +96,7 @@ var _ = Describe("Flags Specs", func() {
 		Ω(output).Should(ContainSubstring("10 Passed"))
 		Ω(output).Should(ContainSubstring("0 Failed"))
 		Ω(output).Should(ContainSubstring("1 Pending"))
-		Ω(output).Should(ContainSubstring("2 Skipped"))
+		Ω(output).Should(ContainSubstring("3 Skipped"))
 	})
 
 	It("should run the race detector when told to", func() {
@@ -108,7 +108,7 @@ var _ = Describe("Flags Specs", func() {
 	})
 
 	It("should randomize tests when told to", func() {
-		session := startGinkgo(pathToTest, "--noColor", "--randomizeAllSpecs", "--seed=21")
+		session := startGinkgo(pathToTest, "--noColor", "--randomizeAllSpecs", "--seed=23")
 		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
 		output := string(session.Out.Contents())
 
@@ -122,7 +122,7 @@ var _ = Describe("Flags Specs", func() {
 		output := string(session.Out.Contents())
 
 		Ω(output).ShouldNot(ContainSubstring("Ran 3 samples:"), "has a measurement")
-		Ω(output).Should(ContainSubstring("3 Skipped"))
+		Ω(output).Should(ContainSubstring("4 Skipped"))
 	})
 
 	It("should watch for slow specs", func() {
@@ -161,6 +161,18 @@ var _ = Describe("Flags Specs", func() {
 		Ω(output).Should(ContainSubstring("15 Skipped"))
 	})
 
+	Context("with a flaky test", func() {
+		It("should normally fail", func() {
+			session := startGinkgo(pathToTest, "--focus=flaky")
+			Eventually(session).Should(gexec.Exit(1))
+		})
+
+		It("should pass if retries are requested", func() {
+			session := startGinkgo(pathToTest, "--focus=flaky --flakeAttempts=2")
+			Eventually(session).Should(gexec.Exit(0))
+		})
+	})
+
 	It("should perform a dry run when told to", func() {
 		pathToTest = tmpPath("fail")
 		copyIn("fail_fixture", pathToTest)

internal/spec/spec.go

@@ -17,9 +17,10 @@ type Spec struct {
 
 	containers []*containernode.ContainerNode
 
-	state   types.SpecState
-	runTime time.Duration
-	failure types.SpecFailure
+	state            types.SpecState
+	runTime          time.Duration
+	failure          types.SpecFailure
+	previousFailures bool
 }
 
 func New(subject leafnodes.SubjectNode, containers []*containernode.ContainerNode, announceProgress bool) *Spec {
@@ -58,6 +59,10 @@ func (spec *Spec) Passed() bool {
 	return spec.state == types.SpecStatePassed
 }
 
+func (spec *Spec) Flaked() bool {
+	return spec.state == types.SpecStatePassed && spec.previousFailures
+}
+
 func (spec *Spec) Pending() bool {
 	return spec.state == types.SpecStatePending
 }
@@ -109,6 +114,10 @@ func (spec *Spec) ConcatenatedString() string {
 }
 
 func (spec *Spec) Run(writer io.Writer) {
+	if spec.state == types.SpecStateFailed {
+		spec.previousFailures = true
+	}
+
 	startTime := time.Now()
 	defer func() {
 		spec.runTime = time.Since(startTime)

internal/spec/spec_test.go

@@ -167,6 +167,29 @@ var _ = Describe("Spec", func() {
 		})
 	})
 
+	Describe("Flaked", func() {
+		It("should work if Run is called twice and gets different results", func() {
+			i := 0
+			spec := New(newItWithBody("flaky it", func() {
+				i++
+				if i == 1 {
+					failer.Fail("oops", codeLocation)
+				}
+			}), containers(), false)
+			spec.Run(buffer)
+			Ω(spec.Passed()).Should(BeFalse())
+			Ω(spec.Failed()).Should(BeTrue())
+			Ω(spec.Flaked()).Should(BeFalse())
+			Ω(spec.Summary("").State).Should(Equal(types.SpecStateFailed))
+			Ω(spec.Summary("").Failure.Message).Should(Equal("oops"))
+			spec.Run(buffer)
+			Ω(spec.Passed()).Should(BeTrue())
+			Ω(spec.Failed()).Should(BeFalse())
+			Ω(spec.Flaked()).Should(BeTrue())
+			Ω(spec.Summary("").State).Should(Equal(types.SpecStatePassed))
+		})
+	})
+
 	Describe("Failed", func() {
 		It("should be failed if the failure was panic", func() {
 			spec := New(newItWithBody("panicky it", func() {

internal/specrunner/spec_runner.go

@@ -137,21 +137,20 @@ func (runner *SpecRunner) runSpecs() bool {
 		if skipRemainingSpecs {
 			spec.Skip()
 		}
-		runner.reportSpecWillRun(spec.Summary(runner.suiteID))
 
 		if !spec.Skipped() && !spec.Pending() {
-			runner.runningSpec = spec
-			spec.Run(runner.writer)
-			runner.runningSpec = nil
-			if spec.Failed() {
+			if passed := runner.runSpec(spec); !passed {
 				suiteFailed = true
 			}
 		} else if spec.Pending() && runner.config.FailOnPending {
+			runner.reportSpecWillRun(spec.Summary(runner.suiteID))
 			suiteFailed = true
+			runner.reportSpecDidComplete(spec.Summary(runner.suiteID), spec.Failed())
+		} else {
+			runner.reportSpecWillRun(spec.Summary(runner.suiteID))
+			runner.reportSpecDidComplete(spec.Summary(runner.suiteID), spec.Failed())
 		}
 
-		runner.reportSpecDidComplete(spec.Summary(runner.suiteID), spec.Failed())
-
 		if spec.Failed() && runner.config.FailFast {
 			skipRemainingSpecs = true
 		}
@@ -160,6 +159,26 @@ func (runner *SpecRunner) runSpecs() bool {
 	return !suiteFailed
 }
 
+func (runner *SpecRunner) runSpec(spec *spec.Spec) (passed bool) {
+	maxAttempts := 1
+	if runner.config.FlakeAttempts > 0 {
+		// uninitialized configs count as 1
+		maxAttempts = runner.config.FlakeAttempts
+	}
+
+	for i := 0; i < maxAttempts; i++ {
+		runner.reportSpecWillRun(spec.Summary(runner.suiteID))
+		runner.runningSpec = spec
+		spec.Run(runner.writer)
+		runner.runningSpec = nil
+		runner.reportSpecDidComplete(spec.Summary(runner.suiteID), spec.Failed())
+		if !spec.Failed() {
+			return true
+		}
+	}
+	return false
+}
+
 func (runner *SpecRunner) CurrentSpecSummary() (*types.SpecSummary, bool) {
 	if runner.runningSpec == nil {
 		return nil, false
@@ -300,6 +319,10 @@ func (runner *SpecRunner) summary(success bool) *types.SuiteSummary {
 		return ex.Passed()
 	})
 
+	numberOfFlakedSpecs := runner.countSpecsSatisfying(func(ex *spec.Spec) bool {
+		return ex.Flaked()
+	})
+
 	numberOfFailedSpecs := runner.countSpecsSatisfying(func(ex *spec.Spec) bool {
 		return ex.Failed()
 	})
@@ -320,5 +343,6 @@ func (runner *SpecRunner) summary(success bool) *types.SuiteSummary {
 		NumberOfSkippedSpecs:               numberOfSkippedSpecs,
 		NumberOfPassedSpecs:                numberOfPassedSpecs,
 		NumberOfFailedSpecs:                numberOfFailedSpecs,
+		NumberOfFlakedSpecs:                numberOfFlakedSpecs,
 	}
 }