From b6bf60ce135c78db973c908e209487a50e52b1da Mon Sep 17 00:00:00 2001 From: Vinh Nguyen <36858281+nhv96@users.noreply.github.com> Date: Sat, 29 Mar 2025 20:49:16 +0700 Subject: [PATCH 1/4] Implemet retry flaky steps (#1) * retry failed pickles * revert some changes * use example steps from cck * update example * implement retry with godog.ErrRetry * unit test for retry flaky * Update flaky feature description * add --retry to CLI option * update test * update test for --retry cli option * update test * handle err without printing retry steps * storage methods to upsert step result * rename wording * revert change * update feature test (will remove this feature) * revert unused files * revert unused files * improve readability * remove unused code * cleanup --- _examples/retry-flaky/features/retry.feature | 19 +++ _examples/retry-flaky/godogs.go | 3 + _examples/retry-flaky/godogs_test.go | 57 ++++++++ internal/flags/flags.go | 2 + internal/flags/flags_test.go | 4 + internal/flags/options.go | 3 + internal/formatters/fmt.go | 1 + internal/models/results.go | 9 ++ internal/storage/storage.go | 23 +++- retry_flaky_test.go | 131 +++++++++++++++++++ run.go | 4 + suite.go | 40 ++++++ 12 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 _examples/retry-flaky/features/retry.feature create mode 100644 _examples/retry-flaky/godogs.go create mode 100644 _examples/retry-flaky/godogs_test.go create mode 100644 retry_flaky_test.go diff --git a/_examples/retry-flaky/features/retry.feature b/_examples/retry-flaky/features/retry.feature new file mode 100644 index 00000000..6d3955d0 --- /dev/null +++ b/_examples/retry-flaky/features/retry.feature @@ -0,0 +1,19 @@ +Feature: Godog should be able to retry flaky tests + In order to help Go developers deal with flaky tests + As a test suite + I need to be able to return godog.Err to mark which steps should be retry + + Scenario: Test cases that pass aren't retried + Given a step that always passes + + Scenario: Test cases that fail are retried if within the limit + Given a step that passes the second time + + Scenario: Test cases that fail will continue to retry up to the limit + Given a step that passes the third time + + Scenario: Test cases won't retry after failing more than the limit + Given a step that always fails + + Scenario: Test cases won't retry when the status is UNDEFINED + Given a non-existent step diff --git a/_examples/retry-flaky/godogs.go b/_examples/retry-flaky/godogs.go new file mode 100644 index 00000000..6d93f352 --- /dev/null +++ b/_examples/retry-flaky/godogs.go @@ -0,0 +1,3 @@ +package main + +func main() { /* usual main func */ } diff --git a/_examples/retry-flaky/godogs_test.go b/_examples/retry-flaky/godogs_test.go new file mode 100644 index 00000000..f6faebee --- /dev/null +++ b/_examples/retry-flaky/godogs_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "context" + "fmt" + "testing" + + "github.com/cucumber/godog" + "github.com/stretchr/testify/assert" +) + +func Test_RetryFlaky(t *testing.T) { + suite := godog.TestSuite{ + Name: "retry flaky tests", + ScenarioInitializer: InitializeScenario, + Options: &godog.Options{ + Format: "pretty", + Paths: []string{"features/retry.feature"}, + MaxRetries: 2, + }, + } + + assert.Equal(t, 0, suite.Run()) +} + +func InitializeScenario(sc *godog.ScenarioContext) { + sc.Step(`^a step that always passes`, func(ctx context.Context) (context.Context, error) { + return ctx, nil + }) + + secondTimePass := 0 + sc.Step(`^a step that passes the second time`, func(ctx context.Context) (context.Context, error) { + secondTimePass++ + if secondTimePass < 2 { + return ctx, fmt.Errorf("unexpected network connection, %w", godog.ErrRetry) + } + return ctx, nil + }) + + thirdTimePass := 0 + sc.Step(`^a step that passes the third time`, func(ctx context.Context) (context.Context, error) { + thirdTimePass++ + if thirdTimePass < 3 { + return ctx, fmt.Errorf("unexpected network connection, %w", godog.ErrRetry) + } + return ctx, nil + }) + + fifthTimePass := 0 + sc.Step(`^a step that always fails`, func(ctx context.Context) (context.Context, error) { + fifthTimePass++ + if fifthTimePass < 5 { + return ctx, fmt.Errorf("must fail, %w", godog.ErrRetry) + } + return ctx, nil + }) +} diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 1bd67e59..50831255 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -46,4 +46,6 @@ built-in formatters are: specify SEED to reproduce the shuffling from a previous run --random=5738`) flagSet.Lookup(prefix + "random").NoOptDefVal = "-1" + + flagSet.IntVar(&opts.MaxRetries, prefix+"retry", opts.MaxRetries, "retry n times when a step encounter error") } diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go index de133335..0308c8fc 100644 --- a/internal/flags/flags_test.go +++ b/internal/flags/flags_test.go @@ -25,6 +25,7 @@ func Test_BindFlagsShouldRespectFlagDefaults(t *testing.T) { assert.False(t, opts.Strict) assert.False(t, opts.NoColors) assert.Equal(t, int64(0), opts.Randomize) + assert.Equal(t, 0, opts.MaxRetries) } func Test_BindFlagsShouldRespectFlagOverrides(t *testing.T) { @@ -37,6 +38,7 @@ func Test_BindFlagsShouldRespectFlagOverrides(t *testing.T) { Strict: true, NoColors: true, Randomize: 11, + MaxRetries: 1, } flagSet := pflag.FlagSet{} @@ -51,6 +53,7 @@ func Test_BindFlagsShouldRespectFlagOverrides(t *testing.T) { "--optOverrides.strict=false", "--optOverrides.no-colors=false", "--optOverrides.random=2", + "--optOverrides.retry=3", }) assert.Equal(t, "junit", opts.Format) @@ -61,4 +64,5 @@ func Test_BindFlagsShouldRespectFlagOverrides(t *testing.T) { assert.False(t, opts.Strict) assert.False(t, opts.NoColors) assert.Equal(t, int64(2), opts.Randomize) + assert.Equal(t, 3, opts.MaxRetries) } diff --git a/internal/flags/options.go b/internal/flags/options.go index 40acea65..f43ac6fc 100644 --- a/internal/flags/options.go +++ b/internal/flags/options.go @@ -80,6 +80,9 @@ type Options struct { // ShowHelp enables suite to show CLI flags usage help and exit. ShowHelp bool + + // MaxRetries is used to retry the number of time when a step is failed. Default is 0 retry. + MaxRetries int } type Feature struct { diff --git a/internal/formatters/fmt.go b/internal/formatters/fmt.go index 3efe1f46..b6ccdad7 100644 --- a/internal/formatters/fmt.go +++ b/internal/formatters/fmt.go @@ -37,6 +37,7 @@ var ( undefined = models.Undefined pending = models.Pending ambiguous = models.Ambiguous + retry = models.Retry ) type sortFeaturesByName []*models.Feature diff --git a/internal/models/results.go b/internal/models/results.go index 9c7f98d7..4c705807 100644 --- a/internal/models/results.go +++ b/internal/models/results.go @@ -37,6 +37,10 @@ type PickleStepResult struct { Def *StepDefinition Attachments []PickleAttachment + + // RunAttempt tells of what run attempt this step result belongs to. + // Default is 1 since every run is the first run. + RunAttempt int } // NewStepResult ... @@ -55,6 +59,7 @@ func NewStepResult( PickleStepID: pickleStepID, Def: match, Attachments: attachments, + RunAttempt: 1, } } @@ -74,6 +79,8 @@ const ( Pending // Ambiguous ... Ambiguous + // Retry ... + Retry ) // Color ... @@ -105,6 +112,8 @@ func (st StepResultStatus) String() string { return "pending" case Ambiguous: return "ambiguous" + case Retry: + return "retry" default: return "unknown" } diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 72b7e86f..7126f752 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -217,7 +217,7 @@ func (s *Storage) MustGetPickleResults() (prs []models.PickleResult) { return prs } -// MustGetPickleStepResult will retrieve a pickle strep result by id and panic on error. +// MustGetPickleStepResult will retrieve a pickle step result by id and panic on error. func (s *Storage) MustGetPickleStepResult(id string) models.PickleStepResult { v := s.mustFirst(tablePickleStepResult, tablePickleStepResultIndexPickleStepID, id) return v.(models.PickleStepResult) @@ -279,6 +279,15 @@ func (s *Storage) MustGetFeatures() (fs []*models.Feature) { return } +// GetPickleStepResultByStepID will retrieve a pickle step result by id and panic on error. +func (s *Storage) GetPickleStepResultByStepID(id string) models.PickleStepResult { + v := s.first(tablePickleStepResult, tablePickleStepResultIndexPickleStepID, id) + if v != nil { + return v.(models.PickleStepResult) + } + return models.PickleStepResult{} +} + type stepDefinitionMatch struct { StepID string StepDefinition *models.StepDefinition @@ -336,3 +345,15 @@ func (s *Storage) mustGet(table, index string, args ...interface{}) memdb.Result return it } + +func (s *Storage) first(table, index string, args ...interface{}) interface{} { + txn := s.db.Txn(readMode) + defer txn.Abort() + + v, err := txn.First(table, index, args...) + if err != nil { + panic(err) + } + + return v +} diff --git a/retry_flaky_test.go b/retry_flaky_test.go new file mode 100644 index 00000000..be47bc49 --- /dev/null +++ b/retry_flaky_test.go @@ -0,0 +1,131 @@ +package godog + +import ( + "bytes" + "context" + "fmt" + "io" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_RetryFlakySteps(t *testing.T) { + output := new(bytes.Buffer) + + featureContents := []Feature{ + { + Name: "RetryFlaky", + Contents: []byte(` +Feature: retry flaky steps + Scenario: Test cases that pass aren't retried + Given a step that always passes + + Scenario: Test cases that fail are retried if within the limit + Given a step that passes the second time + + Scenario: Test cases that fail will continue to retry up to the limit + Given a step that passes the third time + + Scenario: Test cases won't retry after failing more than the limit + Given a step that always fails + + Scenario: Test cases won't retry when the status is UNDEFINED + Given a non-existent step +`), + }, + } + + opts := Options{ + NoColors: true, + Format: "pretty", + Output: output, + FeatureContents: featureContents, + MaxRetries: 3, + } + + status := TestSuite{ + Name: "retry flaky", + ScenarioInitializer: func(ctx *ScenarioContext) { + ctx.Step(`^a step that always passes`, func(ctx context.Context) (context.Context, error) { + return ctx, nil + }) + + secondTimePass := 0 + ctx.Step(`^a step that passes the second time`, func(ctx context.Context) (context.Context, error) { + secondTimePass++ + if secondTimePass < 2 { + return ctx, fmt.Errorf("unexpected network connection, %w", ErrRetry) + } + return ctx, nil + }) + + thirdTimePass := 0 + ctx.Step(`^a step that passes the third time`, func(ctx context.Context) (context.Context, error) { + thirdTimePass++ + if thirdTimePass < 3 { + return ctx, fmt.Errorf("unexpected network connection, %w", ErrRetry) + } + return ctx, nil + }) + + fifthTimePass := 0 + ctx.Step(`^a step that always fails`, func(ctx context.Context) (context.Context, error) { + fifthTimePass++ + if fifthTimePass < 5 { + return ctx, fmt.Errorf("must fail, %w", ErrRetry) + } + return ctx, nil + }) + }, + Options: &opts, + }.Run() + + const expected = `Feature: retry flaky steps + + Scenario: Test cases that pass aren't retried # RetryFlaky:3 + Given a step that always passes # retry_flaky_test.go:51 -> github.com/cucumber/godog.Test_RetryFlakySteps.func1.1 + + Scenario: Test cases that fail are retried if within the limit # RetryFlaky:6 + Given a step that passes the second time # retry_flaky_test.go:56 -> github.com/cucumber/godog.Test_RetryFlakySteps.func1.2 + + Scenario: Test cases that fail will continue to retry up to the limit # RetryFlaky:9 + Given a step that passes the third time # retry_flaky_test.go:65 -> github.com/cucumber/godog.Test_RetryFlakySteps.func1.3 + + Scenario: Test cases won't retry after failing more than the limit # RetryFlaky:12 + Given a step that always fails # retry_flaky_test.go:74 -> github.com/cucumber/godog.Test_RetryFlakySteps.func1.4 + must fail, retry step + + Scenario: Test cases won't retry when the status is UNDEFINED # RetryFlaky:15 + Given a non-existent step + +--- Failed steps: + + Scenario: Test cases won't retry after failing more than the limit # RetryFlaky:12 + Given a step that always fails # RetryFlaky:13 + Error: must fail, retry step + + +5 scenarios (3 passed, 1 failed, 1 undefined) +5 steps (3 passed, 1 failed, 1 undefined) +0s + +You can implement step definitions for undefined steps with these snippets: + +func aNonexistentStep() error { + return godog.ErrPending +} + +func InitializeScenario(ctx *godog.ScenarioContext) { + ctx.Step(` + "`^a non-existent step$`" + `, aNonexistentStep) +} + +` + + actualOutput, err := io.ReadAll(output) + require.NoError(t, err) + + assert.Equal(t, exitFailure, status) + assert.Equal(t, expected, string(actualOutput)) +} diff --git a/run.go b/run.go index 1231d028..6713c9be 100644 --- a/run.go +++ b/run.go @@ -52,6 +52,8 @@ type runner struct { storage *storage.Storage fmt Formatter + + maxRetries int } func (r *runner) concurrent(rate int) (failed bool) { @@ -69,6 +71,7 @@ func (r *runner) concurrent(rate int) (failed bool) { storage: r.storage, defaultContext: r.defaultContext, testingT: r.testingT, + maxRetries: r.maxRetries, }, } if r.testSuiteInitializer != nil { @@ -283,6 +286,7 @@ func runWithOptions(suiteName string, runner runner, opt Options) int { runner.strict = opt.Strict runner.defaultContext = opt.DefaultContext runner.testingT = opt.TestingT + runner.maxRetries = opt.MaxRetries // store chosen seed in environment, so it could be seen in formatter summary report os.Setenv("GODOG_SEED", strconv.FormatInt(runner.randomSeed, 10)) diff --git a/suite.go b/suite.go index b3a87a0b..983558ef 100644 --- a/suite.go +++ b/suite.go @@ -34,6 +34,9 @@ var ErrPending = fmt.Errorf("step implementation is pending") // ErrSkip should be returned by step definition or a hook if scenario and further steps are to be skipped. var ErrSkip = fmt.Errorf("skipped") +// ErrRetry should be returned by step definition to stop further scenario execution and schedule the step for retries. +var ErrRetry = fmt.Errorf("retry step") + // StepResultStatus describes step result. type StepResultStatus = models.StepResultStatus @@ -50,6 +53,8 @@ const ( StepPending = models.Pending // StepAmbiguous indicates step text matches more than one step def StepAmbiguous = models.Ambiguous + // StepRetry indicates the step that need retry. + StepRetry = models.Retry ) type suite struct { @@ -62,6 +67,7 @@ type suite struct { randomSeed int64 stopOnFailure bool strict bool + maxRetries int defaultContext context.Context testingT *testing.T @@ -195,6 +201,27 @@ func (s *suite) runStep(ctx context.Context, pickle *Scenario, step *Step, scena return } + // the step is marked to retry + if errors.Is(err, ErrRetry) { + var sr models.PickleStepResult + sr = s.storage.GetPickleStepResultByStepID(step.Id) + if sr.PickleID == pickle.Id && sr.PickleStepID == step.Id { + if sr.RunAttempt >= s.maxRetries { + sr.Status = models.Failed + sr.Err = err + + s.storage.MustInsertPickleStepResult(sr) + s.fmt.Failed(pickle, step, match.GetInternalStepDefinition(), err) + } else { + sr.RunAttempt++ + } + } else { + sr = models.NewStepResult(models.Retry, pickle.Id, step.Id, match, pickledAttachments, nil) + } + s.storage.MustInsertPickleStepResult(sr) + return + } + switch { case err == nil: sr := models.NewStepResult(models.Passed, pickle.Id, step.Id, match, pickledAttachments, nil) @@ -574,6 +601,19 @@ func (s *suite) runSteps(ctx context.Context, pickle *Scenario, steps []*Step) ( isLast := i == len(steps)-1 isFirst := i == 0 ctx, stepErr = s.runStep(ctx, pickle, step, scenarioErr, isFirst, isLast) + + if errors.Is(stepErr, ErrRetry) { + allowedAttempts := 0 + + for allowedAttempts < s.maxRetries { + allowedAttempts++ + ctx, stepErr = s.runStep(ctx, pickle, step, scenarioErr, isFirst, isLast) + if stepErr == nil { + break + } + } + } + if scenarioErr == nil || s.shouldFail(stepErr) { scenarioErr = stepErr } From 68e3cd4ad4dfcdbadf4c916ad88a048839c02a32 Mon Sep 17 00:00:00 2001 From: Vinh Nguyen <36858281+nhv96@users.noreply.github.com> Date: Sat, 29 Mar 2025 21:04:25 +0700 Subject: [PATCH 2/4] Update changelog (#2) * retry failed pickles * revert some changes * use example steps from cck * update example * implement retry with godog.ErrRetry * unit test for retry flaky * Update flaky feature description * add --retry to CLI option * update test * update test for --retry cli option * update test * handle err without printing retry steps * storage methods to upsert step result * rename wording * revert change * update feature test (will remove this feature) * revert unused files * revert unused files * improve readability * remove unused code * cleanup * update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5845d207..a4b6cc2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt ## Unreleased +### Added +- Support retry flaky steps with `ErrRetry` error - ([684](https://github.com/cucumber/godog/pull/684) - [nhv2796](https://github.com/nhv2796)) +- CLI option to specify number of retry attempts - ([684](https://github.com/cucumber/godog/pull/684) - [nhv2796](https://github.com/nhv2796)) + ### Added - Step text is added to "step is undefined" error - ([669](https://github.com/cucumber/godog/pull/669) - [vearutop](https://github.com/vearutop)) From 5ce208b094cddd637f4af809b6b5947fdd193651 Mon Sep 17 00:00:00 2001 From: VinhNguyenHoang Date: Sun, 13 Apr 2025 21:31:52 +0700 Subject: [PATCH 3/4] make example less obfuscating --- _examples/retry-flaky/godogs_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/_examples/retry-flaky/godogs_test.go b/_examples/retry-flaky/godogs_test.go index f6faebee..5178558a 100644 --- a/_examples/retry-flaky/godogs_test.go +++ b/_examples/retry-flaky/godogs_test.go @@ -2,7 +2,7 @@ package main import ( "context" - "fmt" + "errors" "testing" "github.com/cucumber/godog" @@ -20,7 +20,8 @@ func Test_RetryFlaky(t *testing.T) { }, } - assert.Equal(t, 0, suite.Run()) + // expect it to fail + assert.Equal(t, 1, suite.Run()) } func InitializeScenario(sc *godog.ScenarioContext) { @@ -32,7 +33,7 @@ func InitializeScenario(sc *godog.ScenarioContext) { sc.Step(`^a step that passes the second time`, func(ctx context.Context) (context.Context, error) { secondTimePass++ if secondTimePass < 2 { - return ctx, fmt.Errorf("unexpected network connection, %w", godog.ErrRetry) + return ctx, godog.ErrRetry } return ctx, nil }) @@ -41,17 +42,12 @@ func InitializeScenario(sc *godog.ScenarioContext) { sc.Step(`^a step that passes the third time`, func(ctx context.Context) (context.Context, error) { thirdTimePass++ if thirdTimePass < 3 { - return ctx, fmt.Errorf("unexpected network connection, %w", godog.ErrRetry) + return ctx, godog.ErrRetry } return ctx, nil }) - fifthTimePass := 0 sc.Step(`^a step that always fails`, func(ctx context.Context) (context.Context, error) { - fifthTimePass++ - if fifthTimePass < 5 { - return ctx, fmt.Errorf("must fail, %w", godog.ErrRetry) - } - return ctx, nil + return ctx, errors.New("must fail") }) } From fd83073e4fb0d063a69d9fe1598a83649f3cc3a2 Mon Sep 17 00:00:00 2001 From: VinhNguyenHoang Date: Sun, 13 Apr 2025 21:32:06 +0700 Subject: [PATCH 4/4] update text message --- internal/flags/flags.go | 2 +- internal/flags/options.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 50831255..9c5290d1 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -47,5 +47,5 @@ specify SEED to reproduce the shuffling from a previous run --random=5738`) flagSet.Lookup(prefix + "random").NoOptDefVal = "-1" - flagSet.IntVar(&opts.MaxRetries, prefix+"retry", opts.MaxRetries, "retry n times when a step encounter error") + flagSet.IntVar(&opts.MaxRetries, prefix+"retry", opts.MaxRetries, "specify the number of times to retry failing tests (default: 0)") } diff --git a/internal/flags/options.go b/internal/flags/options.go index f43ac6fc..3bfaa435 100644 --- a/internal/flags/options.go +++ b/internal/flags/options.go @@ -81,7 +81,7 @@ type Options struct { // ShowHelp enables suite to show CLI flags usage help and exit. ShowHelp bool - // MaxRetries is used to retry the number of time when a step is failed. Default is 0 retry. + // MaxRetries is the number of times you can retry failing tests. Default is 0 retry. MaxRetries int }