Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduction of 'MustPassRepeatedly' decorator #1051

Merged
merged 9 commits into from Oct 19, 2022
6 changes: 3 additions & 3 deletions decorator_dsl.go
Expand Up @@ -21,12 +21,12 @@ You can learn more about decorators here: https://onsi.github.io/ginkgo/#decorat
type FlakeAttempts = internal.FlakeAttempts

/*
RepeatAttempts(uint N) is a decorator that allows you to repeat the execution of individual specs or spec containers. Ginkgo will run them up to `N` times until they fail.
MustPassRepeatedly(uint N) is a decorator that allows you to repeat the execution of individual specs or spec containers. Ginkgo will run them up to `N` times until they fail.

You can learn more here: https://onsi.github.io/ginkgo/#the-repeatattempts-decorator
You can learn more here: https://onsi.github.io/ginkgo/#the-mustpassrepeatedly-decorator
You can learn more about decorators here: https://onsi.github.io/ginkgo/#decorator-reference
*/
type RepeatAttempts = internal.RepeatAttempts
type MustPassRepeatedly = internal.MustPassRepeatedly

/*
Focus is a decorator that allows you to mark a spec or container as focused. Identical to FIt and FDescribe.
Expand Down
21 changes: 11 additions & 10 deletions docs/index.md
Expand Up @@ -2485,11 +2485,11 @@ One quick note on `--repeat`: when you invoke `ginkgo --repeat=N` Ginkgo will ru

Both `--until-it-fails` and `--repeat` help you identify flaky specs early. Doing so will help you debug flaky specs while the context that introduced them is fresh.

A more granular approach to repeating tests is by decorating individual subject or container nodes with the RepeatAttempts(N) decorator:
A more granular approach to repeating tests is by decorating individual subject or container nodes with the MustPassRepeatedly(N) decorator:

```go
Describe("Storing books", func() {
It("can save books to the central library", RepeatAttempts(3), func() {
It("can save books to the central library", MustPassRepeatedly(3), func() {
// this spec has been marked and will be retried up to 3 times
})

Expand Down Expand Up @@ -4839,13 +4839,13 @@ With this setup, `"is flaky"` and `"is also flaky"` will run up to 3 times. `"i

If `ginkgo --flake-attempts=N` is set the value passed in by the CLI will override all the decorated values. Every test will now run up to `N` times.

#### The RepeatAttempts Decorator
The `RepeatAttempts(uint)` decorator applies to container and subject nodes. It is an error to apply `RepeatAttempts` to a setup node.
#### The MustPassRepeatedly Decorator
The `MustPassRepeatedly(uint)` decorator applies to container and subject nodes. It is an error to apply `MustPassRepeatedly` to a setup node.

`RepeatAttempts` allows the user to flag specific tests or groups of tests for debbuging flaky tests. Ginkgo will run tests up to the number of times specified in `RepeatAttempts` until they fail. For example:
`MustPassRepeatedly` allows the user to flag specific tests or groups of tests for debbuging flaky tests. Ginkgo will run tests up to the number of times specified in `MustPassRepeatedly` until they fail. For example:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a nitpick (sorry!) but I've been trying to call "tests" in Ginkgo "specs" throughout the documentation (though I'm sure I've missed quite a few!). Would you be up for changing the language here from test/tests to spec/specs?

The rationale is to distinguish Ginkgo specs from regular go tests - but it's mostly about consistency tbh.


```go
Describe("repeated tests", RepeatAttempts(3), func() {
Describe("repeated tests", MustPassRepeatedly(3), func() {
It("is retried", func() {
...
})
Expand All @@ -4854,20 +4854,21 @@ Describe("repeated tests", RepeatAttempts(3), func() {
...
})

It("is retried even more", RepeatAttempts(5) func() {
It("is retried even more", MustPassRepeatedly(5) func() {
...
})

It("is retried less", RepeatAttempts(1), func() {
It("is retried less", MustPassRepeatedly(1), func() {
...
})
})
```

With this setup, `"is retried"` and `"is also retried"` will run up to 3 times. `"is retried even more"` will run up to 5 times. `"is retried less"` will run only once. Note that if multiple `RepeatAttempts` appear in a spec's hierarchy, the most deeply nested `RepeatAttempts` wins. If multiple `RepeatAttempts` are passed into a given node, the last one wins.
With this setup, `"is retried"` and `"is also retried"` will run up to 3 times. `"is retried even more"` will run up to 5 times. `"is retried less"` will run only once. Note that if multiple `MustPassRepeatedly` appear in a spec's hierarchy, the most deeply nested `MustPassRepeatedly` wins. If multiple `MustPassRepeatedly` are passed into a given node, the last one wins.

The `ginkgo --repeat=N` value passed in by the CLI has no relation with the `RepeatAttempts` decorator. If the `--repeat` CLI flag is used and a container or subject node also contains the `RepeatAttempts` decorator, then the test will run up to `N*R` times, where `N` is the values passed to the `--repeat` CLI flag and `R` is the value passed to the RepeatAttempts decorator.
The `ginkgo --repeat=N` value passed in by the CLI has no relation with the `MustPassRepeatedly` decorator. If the `--repeat` CLI flag is used and a container or subject node also contains the `MustPassRepeatedly` decorator, then the test will run up to `N*R` times, where `N` is the values passed to the `--repeat` CLI flag and `R` is the value passed to the MustPassRepeatedly decorator.

If the `MustPassRepeatedly` decorator is set, it will override the `ginkgo --flake-attempts=N` CLI config. The tests that do not contain the `MustPassRepeatedly(R)` decorator will still run up to `N` times, in accordance to the `ginkgo --flake-attempts=N` CLI config.

#### The SuppressProgressOutput Decorator

Expand Down
2 changes: 1 addition & 1 deletion dsl/decorators/decorators_dsl.go
Expand Up @@ -18,7 +18,7 @@ import (

type Offset = ginkgo.Offset
type FlakeAttempts = ginkgo.FlakeAttempts
type RepeatAttempts = ginkgo.RepeatAttempts
type MustPassRepeatedly = ginkgo.MustPassRepeatedly
type Labels = ginkgo.Labels
type PollProgressAfter = ginkgo.PollProgressAfter
type PollProgressInterval = ginkgo.PollProgressInterval
Expand Down
@@ -0,0 +1,34 @@
package decorations_fixture_test

import (
"fmt"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestDecorationsFixture(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "DecorationsFixture Suite")
}

var countFlake = 0
var countRepeat = 0

var _ = Describe("some decorated tests", func() {
It("passes eventually", func() {
countFlake += 1
if countFlake < 3 {
Fail("fail")
}
}, FlakeAttempts(3))

// how to/should we test negative test cases?
It("fails eventually", func() {
countRepeat += 1
if countRepeat >= 3 {
Fail(fmt.Sprintf("failed on attempt #%d", countRepeat))
}
}, MustPassRepeatedly(3))
})
@@ -0,0 +1,19 @@
package invalid_decorations_flakeattempts_mustpassrepeatedly_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestInvalidDecorations(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "InvalidDecorations Suite - MustPassRepeatedly and FlakeAttempts")
}

var _ = Describe("invalid decorators: mustpassrepeatedly and flakeattempts", FlakeAttempts(3), MustPassRepeatedly(3), func() {
It("never runs", func() {

})
})

This file was deleted.

Expand Up @@ -13,7 +13,7 @@ func TestDecorationsFixture(t *testing.T) {
}

var countFlake = 0
// var countRepeat = 0
var countRepeat = 0

var _ = Describe("some decorated tests", func() {
Describe("focused", Focus, func() {
Expand All @@ -24,21 +24,6 @@ var _ = Describe("some decorated tests", func() {

})

FIt("passes eventually", func() {
countFlake += 1
if countFlake < 3 {
Fail("fail")
}
}, FlakeAttempts(3))

// how to/should we test negative test cases?
// FIt("fails eventually", func() {
// countRepeat += 1
// if countRepeat >=3 {
// Fail("fail")
// }
// }, RepeatAttempts(3))

It("focused it", Focus, func() {
Ω(true).Should(BeTrue())
})
Expand Down
1 change: 0 additions & 1 deletion integration/_fixtures/flags_fixture/flags_test.go
Expand Up @@ -85,7 +85,6 @@ var _ = Describe("Testing various flags", func() {
})
})

// what is this testing? This test and the previous are being skipped
Describe("a flaky test", func() {
runs := 0
It("should only pass the second time it's run", func() {
Expand Down
23 changes: 18 additions & 5 deletions integration/decorations_test.go
Expand Up @@ -13,8 +13,8 @@ var _ = Describe("Decorations", func() {
fm.MountFixture("decorations")
})

It("processes the various decorations", func() {
session := startGinkgo(fm.PathTo("decorations"), "-vv", "--no-color")
It("processes the Offset, Focus and Pending decorations", func() {
session := startGinkgo(fm.PathTo("decorations", "offset_focus_pending"), "-vv", "--no-color")
Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))

out := string(session.Out.Contents())
Expand All @@ -24,18 +24,31 @@ some decorated tests
.*decorations_fixture_suite_test.go:\d+
pending it`,
))

Ω(out).ShouldNot(ContainSubstring("never_see_this_file"))
})

It("processes the FlakeAttempts and the MustPassRepeatedly decorations", func() {
session := startGinkgo(fm.PathTo("decorations", "flaky_repeated"), "-vv", "--no-color")
Eventually(session).Should(gexec.Exit(1))

Ω(session).Should(gbytes.Say("Ginkgo: Attempt #1 Failed. Retrying..."))
Ω(session).Should(gbytes.Say("Ginkgo: Attempt #2 Failed. Retrying..."))

Ω(session).Should(gbytes.Say("Ginkgo: Attempt #1 Passed. Repeating..."))
Ω(session).Should(gbytes.Say("Ginkgo: Attempt #2 Passed. Repeating..."))
Ω(session).Should(gbytes.Say("failed on attempt #3"))
})

It("exits with a clear error if decorations are misconfigured - focus and pending error", func() {
session := startGinkgo(fm.PathTo("decorations", "invalid_decorations_focused_pending"), "-v", "--no-color")
Eventually(session).Should(gexec.Exit(1))
Ω(session).Should(gbytes.Say("Invalid Combination of Decorators: Focused and Pending"))
})

It("exits with a clear error if decorations are misconfigured - flakeattempts and repeatattempts error", func() {
session := startGinkgo(fm.PathTo("decorations", "invalid_decorations_flakeattempts_repeatattempts"), "-v", "--no-color")
It("exits with a clear error if decorations are misconfigured - flakeattempts and mustpassrepeatedly error", func() {
session := startGinkgo(fm.PathTo("decorations", "invalid_decorations_flakeattempts_mustpassrepeatedly"), "-v", "--no-color")
Eventually(session).Should(gexec.Exit(1))
Ω(session).Should(gbytes.Say("Invalid Combination of Decorators: FlakeAttempts and RepeatAttempts"))
Ω(session).Should(gbytes.Say("Invalid Combination of Decorators: FlakeAttempts and MustPassRepeatedly"))
})
})
24 changes: 11 additions & 13 deletions internal/group.go
Expand Up @@ -119,8 +119,8 @@ func (g *group) initialReportForSpec(spec Spec) types.SpecReport {
ParallelProcess: g.suite.config.ParallelProcess,
IsSerial: spec.Nodes.HasNodeMarkedSerial(),
IsInOrderedContainer: !spec.Nodes.FirstNodeMarkedOrdered().IsZero(),
IsFlakeAttempts: spec.Nodes.HasSetFlakeAttempts(),
IsRepeatAttempts: spec.Nodes.HasSetRepeatAttempts(),
MaxFlakeAttempts: spec.Nodes.GetMaxFlakeAttempts(),
MaxMustPassRepeatedly: spec.Nodes.GetMaxMustPassRepeatedly(),
}
}

Expand Down Expand Up @@ -306,21 +306,19 @@ func (g *group) run(specs Specs) {
var maxAttempts = 1
var multipleExecutionDecorator interface{}

if g.suite.currentSpecReport.IsFlakeAttempts {
if g.suite.currentSpecReport.MaxFlakeAttempts > 0 {
multipleExecutionDecorator = reflect.TypeOf(FlakeAttempts(0))
maxAttempts = max(1, spec.FlakeAttempts())
}
if g.suite.config.FlakeAttempts > 0 {
multipleExecutionDecorator = reflect.TypeOf(FlakeAttempts(0))
maxAttempts = g.suite.config.FlakeAttempts
g.suite.currentSpecReport.IsFlakeAttempts = true
g.suite.currentSpecReport.MaxFlakeAttempts = maxAttempts
}

if g.suite.currentSpecReport.IsRepeatAttempts {
multipleExecutionDecorator = reflect.TypeOf(RepeatAttempts(0))
maxAttempts = max(1, spec.RepeatAttempts())
} else if g.suite.config.FlakeAttempts > 0 {
//What should be the behavior when --flakeattempts is defined in CLI and the RepeatAttemps decorator is used?
if g.suite.currentSpecReport.MaxMustPassRepeatedly > 0 {
multipleExecutionDecorator = reflect.TypeOf(MustPassRepeatedly(0))
maxAttempts = max(1, spec.MustPassRepeatedly())
}

maxAttemptForLoop:
Expand All @@ -332,8 +330,8 @@ func (g *group) run(specs Specs) {
switch t := multipleExecutionDecorator; {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than tracking multipleExecutionDecorator and doing a type switch could these be if g.suite.currentSpecReport.MaxFlakeAttampts > 0 and else if g.suite.currentSpecReport.MaxMustPassRepeatedly > 0?

case t == reflect.TypeOf(FlakeAttempts(0)):
fmt.Fprintf(g.suite.writer, "\nGinkgo: Attempt #%d Failed. Retrying...\n", attempt)
case t == reflect.TypeOf(RepeatAttempts(0)):
fmt.Fprintf(g.suite.writer, "\nGinkgo: Attempt #%d Passed. Retrying...\n", attempt)
case t == reflect.TypeOf(MustPassRepeatedly(0)):
fmt.Fprintf(g.suite.writer, "\nGinkgo: Attempt #%d Passed. Repeating...\n", attempt)
}
}

Expand All @@ -349,8 +347,8 @@ func (g *group) run(specs Specs) {
if g.suite.currentSpecReport.State.Is(types.SpecStatePassed | types.SpecStateSkipped | types.SpecStateAborted | types.SpecStateInterrupted) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks right to me.

break maxAttemptForLoop
}
case t == reflect.TypeOf(RepeatAttempts(0)):
if g.suite.currentSpecReport.State.Is(types.SpecStateFailed | types.SpecStateSkipped | types.SpecStateAborted | types.SpecStateInterrupted) {
case t == reflect.TypeOf(MustPassRepeatedly(0)):
if g.suite.currentSpecReport.State.Is(types.SpecStateFailureStates | types.SpecStateSkipped) {
break maxAttemptForLoop
}
}
Expand Down
12 changes: 6 additions & 6 deletions internal/internal_integration/decorations_test.go
Expand Up @@ -34,18 +34,18 @@ var _ = Describe("Decorations test", func() {
It("flaky-skips", FlakeAttempts(3), rt.T("flaky-skips", func() {
Skip("skip")
}))
It("repeat", RepeatAttempts(4), rt.T("repeat", func() {
It("repeat", MustPassRepeatedly(4), rt.T("repeat", func() {
countRepeat += 1
outputInterceptor.AppendInterceptedOutput("repeats a bit\n")
writer.Println("here we go")
if countRepeat >= 3 {
F("fail")
}
}))
It("repeat-never-fails", RepeatAttempts(2), rt.T("repeat-never-passes", func() {
It("repeat-never-fails", MustPassRepeatedly(2), rt.T("repeat-never-passes", func() {
// F("fail")
}))
It("repeat-skips", RepeatAttempts(3), rt.T("repeat-skips", func() {
It("repeat-skips", MustPassRepeatedly(3), rt.T("repeat-skips", func() {
Skip("skip")
}))
})
Expand Down Expand Up @@ -80,9 +80,9 @@ var _ = Describe("Decorations test", func() {
})
})

Describe("RepeatAttempts", func() {
It("reruns tests until they fail or until the number of repeat attempts is exhausted, but does not rerun skipped tests", func() {
Ω(reporter.Did.Find("repeat")).Should(HaveFailed(NumAttempts(3), CapturedStdOutput("repeats a bit\nrepeats a bit\nrepeats a bit\n"), CapturedGinkgoWriterOutput("here we go\n\nGinkgo: Attempt #1 Passed. Retrying...\nhere we go\n\nGinkgo: Attempt #2 Passed. Retrying...\nhere we go\n")))
Describe("MustPassRepeatedly", func() {
It("reruns tests until they fail or until the number of MustPassRepeatedly attempts is exhausted, but does not rerun skipped tests", func() {
Ω(reporter.Did.Find("repeat")).Should(HaveFailed(NumAttempts(3), CapturedStdOutput("repeats a bit\nrepeats a bit\nrepeats a bit\n"), CapturedGinkgoWriterOutput("here we go\n\nGinkgo: Attempt #1 Passed. Repeating...\nhere we go\n\nGinkgo: Attempt #2 Passed. Repeating...\nhere we go\n")))
Ω(reporter.Did.Find("repeat-never-fails")).Should(HavePassed("passed", NumAttempts(2)))
Ω(reporter.Did.Find("repeat-skips")).Should(HaveBeenSkippedWithMessage("skip", NumAttempts(1)))
})
Expand Down
44 changes: 43 additions & 1 deletion internal/internal_integration/table_test.go
Expand Up @@ -400,7 +400,7 @@ var _ = Describe("Table driven tests", func() {
})
})

Describe("support for decorators", func() {
Describe("support for FlakyAttempts decorators", func() {
BeforeEach(func() {
success, _ := RunFixture("flaky table", func() {
var counter int
Expand Down Expand Up @@ -441,4 +441,46 @@ var _ = Describe("Table driven tests", func() {
Ω(reporter.Did.Find("D")).Should(HavePassed(NumAttempts(3)))
})
})

Describe("support for MustPassRepeatedly decorators", func() {
BeforeEach(func() {
success, _ := RunFixture("Repeated Passes table", func() {
var counter int
var currentSpec string

BeforeEach(func() {
if currentSpec != CurrentSpecReport().LeafNodeText {
counter = 0
currentSpec = CurrentSpecReport().LeafNodeText
}
})

DescribeTable("contrived Repeated Passes table", MustPassRepeatedly(2),
func(passUntil int) {
rt.Run(CurrentSpecReport().LeafNodeText)
counter += 1
if counter >= passUntil {
F("fail")
}
},
Entry("A", 1),
Entry("B", 2),
Entry("C", 3),
Entry("D", []interface{}{MustPassRepeatedly(3), Offset(2)}, 3),
)
})
Ω(success).Should(BeFalse())
})

It("honors the Repeated attempts decorator", func() {
Ω(rt).Should(HaveTracked("A", "B", "B", "C", "C", "D", "D", "D"))
})

It("reports on the specs appropriately", func() {
Ω(reporter.Did.Find("A")).Should(HaveFailed(NumAttempts(1)))
Ω(reporter.Did.Find("B")).Should(HaveFailed(NumAttempts(2)))
Ω(reporter.Did.Find("C")).Should(HavePassed(NumAttempts(2)))
Ω(reporter.Did.Find("D")).Should(HaveFailed(NumAttempts(3)))
})
})
})