diff --git a/README.md b/README.md index aaef261..da837d1 100644 --- a/README.md +++ b/README.md @@ -11,16 +11,16 @@ `gdt` is a testing library that allows test authors to cleanly describe tests in a YAML file. `gdt` reads YAML files that describe a test's assertions and -then builds a set of Golang structures that the standard Golang +then builds a set of Go structures that the standard Go [`testing`](https://golang.org/pkg/testing/) package and standard `go test` tool can execute. ## Introduction -Writing functional tests in Golang can be overly verbose and tedious. When the -code that tests some part of an application is verbose or tedious, then it -becomes difficult to read the tests and quickly understand the assertions the -test is making. +Writing functional tests in Go can be overly verbose and tedious. When the code +that tests some part of an application is verbose or tedious, then it becomes +difficult to read the tests and quickly understand the assertions the test is +making. The more difficult it is to understand the test assertions or the test setups and assumptions, the greater the chance that the test improperly validates the @@ -34,7 +34,7 @@ describe a functional test's **assumptions** and **assertions** in a declarative format. Separating the *description* of a test's assumptions (setup) and assertions -from the Golang code that actually performs the test assertions leads to tests +from the Go code that actually performs the test assertions leads to tests that are easier to read and understand. This allows developers to spend *more time writing code* and less time copy/pasting boilerplate test code. Due to the easier test comprehension, `gdt` also encourages writing greater quality and @@ -138,9 +138,9 @@ var _ = Describe("Books API Types", func() { ``` -This is perfectly fine for simple unit tests of Golang code. However, once the -tests begin to call multiple APIs or packages, the Ginkgo Golang tests start to -get cumbersome. Consider the following example of *functionally* testing the +This is perfectly fine for simple unit tests of Go code. However, once the +tests begin to call multiple APIs or packages, the Ginkgo Go tests start to get +cumbersome. Consider the following example of *functionally* testing the failure modes for a simple HTTP REST API endpoint ([`failure_test.go`](https://github.com/gdt-dev/gdt-examples/blob/main/http/api/failure_test.go)): @@ -256,7 +256,7 @@ var _ = Describe("Books API - GET /books failures", func() { ``` The above test code obscures what is being tested by cluttering the test -assertions with the Golang closures and accessor code. Compare the above with +assertions with the Go closures and accessor code. Compare the above with how `gdt` allows the test author to describe the same assertions ([`failures.yaml`](https://github.com/gdt-dev/gdt-examples/blob/main/http/tests/api/failures.yaml)): @@ -284,7 +284,7 @@ No more closures and boilerplate function code getting in the way of expressing the assertions, which should be the focus of the test. The more intricate the assertions being verified by the test, generally the -more verbose and cumbersome the Golang test code tends to become. First and +more verbose and cumbersome the Go test code tends to become. First and foremost, tests should be *readable*. If they are not readable, then the test's assertions are not *understandable*. And tests that cannot easily be understood are often the source of bit rot and technical debt. Worse, tests that aren't @@ -300,7 +300,7 @@ Consider a Ginkgo test case that checks the following behaviour: * The newly-created book's ID field is a valid UUID * The newly-created book's publisher has an address containing a known state code -A typical implementation of a Ginkgo Golang test might look like this +A typical implementation of a Ginkgo test might look like this ([`create_then_get_test.go`](https://github.com/gdt-dev/gdt-examples/blob/main/http/api/create_then_get_test.go)): ```go @@ -423,8 +423,7 @@ All `gdt` scenarios have the following fields: missing or empty, the filename is used as the name * `description`: (optional) string with longer description of the test file contents -* `defaults`: (optional) is a map, keyed by a plugin name, of default options - and configuration values for that plugin. +* `defaults`: (optional) is a map of default options and configuration values * `fixtures`: (optional) list of strings indicating named fixtures that will be started before any of the tests in the file are run * `skip-if`: (optional) list of [`Spec`][basespec] specializations that will be @@ -433,26 +432,29 @@ All `gdt` scenarios have the following fields: * `tests`: list of [`Spec`][basespec] specializations that represent the runnable test units in the test scenario. -[basespec]: https://github.com/gdt-dev/gdt/blob/2791e11105fd3c36d1f11a7d111e089be7cdc84c/types/spec.go#L27-L44 +[basespec]: https://github.com/gdt-dev/gdt/blob/ecee17249e1fa10147cf9191be0358923da44094/types/spec.go#L30 The scenario's `tests` field is the most important and the [`Spec`][basespec] objects that it contains are the meat of a test scenario. -## `gdt` test spec structure +### `gdt` test spec structure A spec represents a single *action* that is taken and zero or more *assertions* that represent what you expect to see resulting from that action. -Each spec is a specialized class of the base [`Spec`][basespec] that deals with -a particular type of test. For example, there is a `Spec` class called `exec` -that allows you to execute arbitrary commands and assert expected result codes -and output. There is a `Spec` class called `http` that allows you to call an -HTTP URL and assert that the response looks like what you expect. Depending on -how you define your test units, `gdt` will parse the YAML definition into one -of these specialized `Spec` classes. +`gdt` plugins each define a specialized subclass of the base [`Spec`][basespec] +that contains fields that are specific to that type of test. -The base `Spec` class has the following fields (and thus all `Spec` specialized -classes inherit these fields): +For example, there is an [`exec`][exec-plugin] plugin that allows you to +execute arbitrary commands and assert expected result codes and output. There +is an [`http`][http-plugin] that allows you to call an HTTP URL and assert that +the response looks like what you expect. There is a [`kube`][kube-plugin] +plugin that allows you to interact with a Kubernetes API, etc. + +`gdt` examines the YAML file that defines your test scenario and uses these +plugins to parse individual test specs. + +All test specs have the following fields: * `name`: (optional) string describing the test unit. * `description`: (optional) string with longer description of the test unit. @@ -462,24 +464,49 @@ classes inherit these fields): complete within. * `timeout.expected`: a bool indicating that the test unit is expected to not complete before `timeout.after`. This is really only useful in unit testing. +* `retry`: (optional) an object containing retry configurationu for the test + unit. Some plugins will automatically attempt to retry the test action when + an assertion fails. This field allows you to control this retry behaviour for + each individual test. +* `retry.interval`: (optional) a string duration of time that the test plugin + will retry the test action in the event assertions fail. The default interval + for retries is plugin-dependent. +* `retry.attempts`: (optional) an integer indicating the number of times that a + plugin will retry the test action in the event assertions fail. The default + number of attempts for retries is plugin-dependent. +* `retry.exponential`: (optional) a boolean indicating an exponential backoff + should be applied to the retry interval. The default is is plugin-dependent. * `wait` (optional) an object containing [wait information][wait] for the test unit. * `wait.before`: a string duration of time that gdt should wait before executing the test unit's action. * `wait.after`: a string duration of time that gdt should wait after executing the test unit's action. +* `on`: (optional) an object describing actions to take upon certain + conditions. +* `on.fail`: (optional) an object describing an action to take when any + assertion fails for the test action. +* `on.fail.exec`: a string with the exact command to execute upon test + assertion failure. You may execute more than one command but must include the + `on.fail.shell` field to indicate that the command should be run in a shell. +* `on.fail.shell`: (optional) a string with the specific shell to use in executing the + command to run upon test assertion failure. If empty (the default), no shell + is used to execute the command and instead the operating system's `exec` family + of calls is used. +[exec-plugin]: https://github.com/gdt-dev/gdt/tree/ecee17249e1fa10147cf9191be0358923da44094/plugin/exec +[http-plugin]: https://github.com/gdt-dev/http +[kube-plugin]: https://github.com/gdt-dev/kube [timeout]: https://github.com/gdt-dev/gdt/blob/2791e11105fd3c36d1f11a7d111e089be7cdc84c/types/timeout.go#L11-L22 [wait]: https://github.com/gdt-dev/gdt/blob/2791e11105fd3c36d1f11a7d111e089be7cdc84c/types/wait.go#L11-L25 -### `exec` test spec structure +#### `exec` test spec structure -An exec spec is a specialization of the base [`Spec`][basespec] that allows -test authors to execute arbitrary commands and assert that the command results -in an expected result code or output. +The `exec` plugin's test spec allows test authors to execute arbitrary commands and +assert that the command results in an expected result code or output. -The [exec `Spec`][execspec] class has the following fields (in addition to all -the base `Spec` fields listed above): +In addition to all the base `Spec` fields listed above, the `exec` plugin's +test spec also contains these fields: * `exec`: a string with the exact command to execute. You may execute more than one command but must include the `shell` field to indicate that the command @@ -514,17 +541,6 @@ the base `Spec` fields listed above): least one* must be present in `stderr`. * `assert.err.none`: (optional) a string or list of strings of which *none should be present* in `stderr`. -* `on`: (optional) an object describing actions to take upon certain - conditions. -* `on.fail`: (optional) an object describing an action to take when any - assertion fails for the test action. -* `on.fail.exec`: a string with the exact command to execute upon test - assertion failure. You may execute more than one command but must include the - `on.fail.shell` field to indicate that the command should be run in a shell. -* `on.fail.shell`: (optional) a string with the specific shell to use in executing the - command to run upon test assertion failure. If empty (the default), no shell - is used to execute the command and instead the operating system's `exec` family - of calls is used. [execspec]: https://github.com/gdt-dev/gdt/blob/2791e11105fd3c36d1f11a7d111e089be7cdc84c/exec/spec.go#L11-L34 [pipeexpect]: https://github.com/gdt-dev/gdt/blob/2791e11105fd3c36d1f11a7d111e089be7cdc84c/exec/assertions.go#L15-L26 @@ -533,7 +549,7 @@ the base `Spec` fields listed above): `gdt` was inspired by [Gabbi](https://github.com/cdent/gabbi), the excellent Python declarative testing framework. `gdt` tries to bring the same clear, -concise test definitions to the world of Golang functional testing. +concise test definitions to the world of Go functional testing. The Go gopher logo, from which gdt's logo was derived, was created by Renee French. diff --git a/errors/parse.go b/errors/parse.go index 867d13e..3761389 100644 --- a/errors/parse.go +++ b/errors/parse.go @@ -53,14 +53,24 @@ var ( ErrExpectedScalarOrSequence = fmt.Errorf( "%w: expected scalar or sequence of scalars field", ErrParse, ) - // ErrParseTimeout indicates that the timeout specification was not valid. + // ErrExpectedTimeout indicates that the timeout specification was not + // valid. ErrExpectedTimeout = fmt.Errorf( "%w: expected timeout specification", ErrParse, ) - // ErrParseWait indicates that the wait specification was not valid. + // ErrExpectedWait indicates that the wait specification was not valid. ErrExpectedWait = fmt.Errorf( "%w: expected wait specification", ErrParse, ) + // ErrExpectedRetry indicates that the retry specification was not valid. + ErrExpectedRetry = fmt.Errorf( + "%w: expected retry specification", ErrParse, + ) + // ErrInvalidRetryAttempts indicates that the retry attempts was not + // positive. + ErrInvalidRetryAttempts = fmt.Errorf( + "%w: invalid retry attempts", ErrParse, + ) // ErrFileNotFound is returned when a file path does not exist for a // create/apply/delete target. ErrFileNotFound = fmt.Errorf( @@ -158,6 +168,24 @@ func ExpectedWaitAt(node *yaml.Node) error { ) } +// ExpectedRetryAt returns an ErrExpectedRetry error annotated with the +// line/column of the supplied YAML node. +func ExpectedRetryAt(node *yaml.Node) error { + return fmt.Errorf( + "%w at line %d, column %d", + ErrExpectedRetry, node.Line, node.Column, + ) +} + +// InvalidRetryAttempts returns an ErrInvalidRetryAttempts error annotated with +// the line/column of the supplied YAML node. +func InvalidRetryAttempts(node *yaml.Node, attempts int) error { + return fmt.Errorf( + "%w of %d at line %d, column %d", + ErrInvalidRetryAttempts, attempts, node.Line, node.Column, + ) +} + // UnknownSourceType returns an ErrUnknownSourceType error describing the // supplied parameter type. func UnknownSourceType(source interface{}) error { diff --git a/go.mod b/go.mod index f1e8df8..dc2f74c 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.21 require ( github.com/PaesslerAG/jsonpath v0.1.1 + github.com/cenkalti/backoff v2.2.1+incompatible github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.3.0 github.com/samber/lo v1.38.1 diff --git a/go.sum b/go.sum index 22835ec..e4b94e9 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,8 @@ github.com/PaesslerAG/gval v1.0.0/go.mod h1:y/nm5yEyTeX6av0OfKJNp9rBNj2XrGhAf5+v github.com/PaesslerAG/jsonpath v0.1.0/go.mod h1:4BzmtoM/PI8fPO4aQGIusjGxGir2BzcV0grWtFzq1Y8= github.com/PaesslerAG/jsonpath v0.1.1 h1:c1/AToHQMVsduPAa4Vh6xp2U0evy4t8SWp8imEsylIk= github.com/PaesslerAG/jsonpath v0.1.1/go.mod h1:lVboNxFGal/VwW6d9JzIy56bUsYAP6tH/x80vjnCseY= +github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= +github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/plugin/exec/eval.go b/plugin/exec/eval.go index 36040cc..8897972 100644 --- a/plugin/exec/eval.go +++ b/plugin/exec/eval.go @@ -31,9 +31,6 @@ func (s *Spec) Eval(ctx context.Context, t *testing.T) *result.Result { } a := newAssertions(s.Assert, ec, outbuf, errbuf) if !a.OK(ctx) { - for _, fail := range a.Failures() { - t.Error(fail) - } if s.On != nil { if s.On.Fail != nil { outbuf.Reset() diff --git a/plugin/exec/eval_test.go b/plugin/exec/eval_test.go index 4fc024a..b3e9bc9 100644 --- a/plugin/exec/eval_test.go +++ b/plugin/exec/eval_test.go @@ -8,7 +8,10 @@ import ( "bufio" "bytes" "context" + "flag" + "fmt" "os" + "os/exec" "path/filepath" "runtime" "testing" @@ -18,6 +21,8 @@ import ( "github.com/stretchr/testify/require" ) +var failFlag = flag.Bool("fail", false, "run tests expected to fail") + func TestNoExitCodeSimpleCommand(t *testing.T) { require := require.New(t) @@ -157,7 +162,10 @@ func TestContainsNoneOf(t *testing.T) { require.Nil(err) } -func TestSleepTimeout(t *testing.T) { +func TestFailExecSleepTimeout(t *testing.T) { + if !*failFlag { + t.Skip("skipping without -fail flag") + } require := require.New(t) fp := filepath.Join("testdata", "sleep-timeout.yaml") @@ -176,6 +184,22 @@ func TestSleepTimeout(t *testing.T) { require.Nil(err) } +func TestExecSleepTimeout(t *testing.T) { + require := require.New(t) + target := os.Args[0] + failArgs := []string{ + "-test.v", + "-test.run=FailExecSleepTimeout", + "-fail", + } + outerr, err := exec.Command(target, failArgs...).CombinedOutput() + + // The test should have failed... + require.NotNil(err) + debugout := fmt.Sprintf("%s", outerr) + require.Contains(debugout, "assertion failed: timeout exceeded") +} + func TestDebugWriter(t *testing.T) { require := require.New(t) @@ -230,7 +254,10 @@ func TestWait(t *testing.T) { require.Contains(debugout, "wait: 20ms after") } -func TestTimeoutCascade(t *testing.T) { +func TestFailExecTimeoutCascade(t *testing.T) { + if !*failFlag { + t.Skip("skipping without -fail flag") + } require := require.New(t) fp := filepath.Join("testdata", "timeout-cascade.yaml") @@ -244,47 +271,32 @@ func TestTimeoutCascade(t *testing.T) { require.Nil(err) require.NotNil(s) - var b bytes.Buffer - w := bufio.NewWriter(&b) - ctx := gdtcontext.New(gdtcontext.WithDebug(w)) + ctx := gdtcontext.New(gdtcontext.WithDebug()) err = s.Run(ctx, t) - require.Nil(err) - require.False(t.Failed()) - w.Flush() - require.NotEqual(b.Len(), 0) - debugout := b.String() +} + +func TestExecTimeoutCascade(t *testing.T) { + require := require.New(t) + target := os.Args[0] + failArgs := []string{ + "-test.v", + "-test.run=FailExecTimeoutCascade", + "-fail", + } + outerr, err := exec.Command(target, failArgs...).CombinedOutput() + + // The test should have failed... + require.NotNil(err) + + debugout := fmt.Sprintf("%s", outerr) require.Contains(debugout, "using timeout of 500ms (expected: false) [scenario default]") require.Contains(debugout, "using timeout of 20ms (expected: true)") } -// Unfortunately there's not really any good way of testing things like this -// except by manually causing an assertion to fail in the test case and -// checking to see if the `on.fail` action was taken and debug output emitted -// to the console. -// -// When I change the `testdata/on-fail-exec.yaml` file to have a failed -// assertion by changing `assert.out.is` to "dat" instead of "cat", I get the -// correct behaviour: -// -// === RUN TestOnFail -// === RUN TestOnFail/on-fail-exec -// -// action.go:59: exec: echo [cat] -// eval.go:35: assertion failed: not equal: expected dat but got cat -// action.go:59: exec: echo [bad kitty] -// eval.go:46: on.fail.exec: stdout: bad kitty -// -// === NAME TestOnFail -// -// eval_test.go:256: -// Error Trace: /home/jaypipes/src/github.com/gdt-dev/gdt/plugin/exec/eval_test.go:256 -// Error: Should be false -// Test: TestOnFail -// -// --- FAIL: TestOnFail (0.00s) -// -// --- FAIL: TestOnFail/on-fail-exec (0.00s) -func TestOnFail(t *testing.T) { +func TestFailExecOnFail(t *testing.T) { + if !*failFlag { + t.Skip("skipping without -fail flag") + } require := require.New(t) fp := filepath.Join("testdata", "on-fail-exec.yaml") @@ -301,5 +313,22 @@ func TestOnFail(t *testing.T) { ctx := gdtcontext.New(gdtcontext.WithDebug()) err = s.Run(ctx, t) require.Nil(err) - require.False(t.Failed()) +} + +func TestExecOnFail(t *testing.T) { + require := require.New(t) + target := os.Args[0] + failArgs := []string{ + "-test.v", + "-test.run=FailExecOnFail", + "-fail", + } + outerr, err := exec.Command(target, failArgs...).CombinedOutput() + + // The test should have failed... + require.NotNil(err) + + debugout := fmt.Sprintf("%s", outerr) + require.Contains(debugout, "assertion failed: not equal: expected dat but got cat") + require.Contains(debugout, "echo [bad kitty]") } diff --git a/plugin/exec/testdata/on-fail-exec.yaml b/plugin/exec/testdata/on-fail-exec.yaml index 5cfc00f..3ebeced 100644 --- a/plugin/exec/testdata/on-fail-exec.yaml +++ b/plugin/exec/testdata/on-fail-exec.yaml @@ -4,28 +4,7 @@ tests: - exec: echo "cat" assert: out: - is: cat - # Unfortunately there's not really any good way of testing things like this - # except by manually causing an assertion to fail in the test case and checking - # to see if the `on.fail` action was taken and debug output emitted to the - # console. - # - # When I change `assert.out.is` above to "dat" instead of "cat", I get the - # correct behaviour: - # - # === RUN TestOnFail - # === RUN TestOnFail/on-fail-exec - # action.go:59: exec: echo [cat] - # eval.go:35: assertion failed: not equal: expected dat but got cat - # action.go:59: exec: echo [bad kitty] - # eval.go:46: on.fail.exec: stdout: bad kitty - # === NAME TestOnFail - # eval_test.go:256: - # Error Trace: /home/jaypipes/src/github.com/gdt-dev/gdt/plugin/exec/eval_test.go:256 - # Error: Should be false - # Test: TestOnFail - # --- FAIL: TestOnFail (0.00s) - # --- FAIL: TestOnFail/on-fail-exec (0.00s) + is: dat on: fail: exec: echo "bad kitty" diff --git a/scenario/defaults.go b/scenario/defaults.go index efeec9e..68fff91 100644 --- a/scenario/defaults.go +++ b/scenario/defaults.go @@ -14,7 +14,9 @@ import ( const ( // DefaultsKey is the key within the Defaults collection for - // scenario defaults. + // scenario defaults. Note that this isn't exposed in the YAML schema for a + // scenario. It's just used as a way of indicating to the scenario runner + // what was found in the scenario YAML's `defaults` top-level field. DefaultsKey = "gdt.scenario" ) @@ -23,6 +25,9 @@ type Defaults struct { // Timeout has fields that represent the default timeout behaviour and // expectations to use for test specs in the scenario. Timeout *gdttypes.Timeout `yaml:"timeout,omitempty"` + // Retry has fields that represent the default retry behaviour for test + // specs in the scenario. + Retry *gdttypes.Retry `yaml:"retry,omitempty"` } func (d *Defaults) UnmarshalYAML(node *yaml.Node) error { @@ -52,6 +57,27 @@ func (d *Defaults) UnmarshalYAML(node *yaml.Node) error { return err } d.Timeout = to + case "retry": + if valNode.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(valNode) + } + var r *gdttypes.Retry + if err := valNode.Decode(&r); err != nil { + return errors.ExpectedRetryAt(valNode) + } + if r.Attempts != nil { + attempts := *r.Attempts + if attempts < 1 { + return errors.InvalidRetryAttempts(valNode, attempts) + } + } + if r.Interval != "" { + _, err := time.ParseDuration(r.Interval) + if err != nil { + return err + } + } + d.Retry = r default: continue } diff --git a/scenario/parse.go b/scenario/parse.go index 4972ae3..753432e 100644 --- a/scenario/parse.go +++ b/scenario/parse.go @@ -83,6 +83,8 @@ func (s *Scenario) UnmarshalYAML(node *yaml.Node) error { s.Defaults = defaults } } + // We store a lookup to the parsing plugin for each parsed test spec + evalPlugins := map[int]gdttypes.Plugin{} for i := 0; i < len(node.Content); i += 2 { keyNode := node.Content[i] if keyNode.Kind != yaml.ScalarNode { @@ -103,21 +105,24 @@ func (s *Scenario) UnmarshalYAML(node *yaml.Node) error { } base.Index = idx base.Defaults = &defaults - specs := []gdttypes.Evaluable{} + pluginSpecs := map[gdttypes.Plugin][]gdttypes.Evaluable{} for _, p := range plugins { - specs = append(specs, p.Specs()...) + pluginSpecs[p] = p.Specs() } - for _, sp := range specs { - if err := testNode.Decode(sp); err != nil { - if errors.Is(err, gdterrors.ErrUnknownField) { - continue + for plugin, specs := range pluginSpecs { + for _, sp := range specs { + if err := testNode.Decode(sp); err != nil { + if errors.Is(err, gdterrors.ErrUnknownField) { + continue + } + return err } - return err + sp.SetBase(base) + s.Tests = append(s.Tests, sp) + parsed = true + evalPlugins[idx] = plugin + break } - sp.SetBase(base) - s.Tests = append(s.Tests, sp) - parsed = true - break } if !parsed { return gdterrors.UnknownSpecAt(s.Path, valNode) @@ -157,5 +162,6 @@ func (s *Scenario) UnmarshalYAML(node *yaml.Node) error { } } } + s.evalPlugins = evalPlugins return nil } diff --git a/scenario/parse_test.go b/scenario/parse_test.go index c27cec1..8991ea9 100644 --- a/scenario/parse_test.go +++ b/scenario/parse_test.go @@ -131,6 +131,45 @@ func TestBadTimeoutDurationScenario(t *testing.T) { assert.Nil(s) } +func TestBadRetry(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + fp := filepath.Join("testdata", "parse", "fail", "bad-retry.yaml") + f, err := os.Open(fp) + require.Nil(err) + + s, err := scenario.FromReader(f, scenario.WithPath(fp)) + assert.ErrorIs(err, errors.ErrExpectedMap) + assert.Nil(s) +} + +func TestBadRetryAttempts(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + fp := filepath.Join("testdata", "parse", "fail", "bad-retry-attempts.yaml") + f, err := os.Open(fp) + require.Nil(err) + + s, err := scenario.FromReader(f, scenario.WithPath(fp)) + assert.ErrorIs(err, errors.ErrInvalidRetryAttempts) + assert.Nil(s) +} + +func TestBadRetryIntervalDuration(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + fp := filepath.Join("testdata", "parse", "fail", "bad-retry-interval-duration.yaml") + f, err := os.Open(fp) + require.Nil(err) + + s, err := scenario.FromReader(f, scenario.WithPath(fp)) + assert.ErrorContains(err, "invalid duration") + assert.Nil(s) +} + func TestKnownSpec(t *testing.T) { assert := assert.New(t) require := require.New(t) diff --git a/scenario/run.go b/scenario/run.go index 7d0948b..877035d 100644 --- a/scenario/run.go +++ b/scenario/run.go @@ -6,13 +6,17 @@ package scenario import ( "context" + "fmt" "strings" "testing" "time" + "github.com/cenkalti/backoff" + gdtcontext "github.com/gdt-dev/gdt/context" "github.com/gdt-dev/gdt/debug" gdterrors "github.com/gdt-dev/gdt/errors" + "github.com/gdt-dev/gdt/result" gdttypes "github.com/gdt-dev/gdt/types" ) @@ -65,7 +69,11 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error { defer func() { ctx = gdtcontext.PopTrace(ctx) }() - for _, spec := range s.Tests { + for idx, spec := range s.Tests { + plugin := s.evalPlugins[idx] + pinfo := plugin.Info() + pretry := pinfo.Retry + // Create a brand new context that inherits the top-level context's // cancel func. We want to set deadlines for each test spec and if // we mutate the single supplied top-level context, then only the @@ -80,17 +88,91 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error { time.Sleep(wait.BeforeDuration()) } - to := specTimeout(ctx, t, sb.Timeout, scDefaults) + to := getTimeout(ctx, sb.Timeout, scDefaults) if to != nil { var cancel context.CancelFunc specCtx, cancel = context.WithTimeout(specCtx, to.Duration()) defer cancel() } - res := spec.Eval(specCtx, t) - if res.HasRuntimeError() { - rterr = res.RuntimeError() - t.Fatal(rterr) - break + + var res *result.Result + rt := getRetry(ctx, sb.Retry, scDefaults, pretry) + if rt == nil { + // Just evaluate the test spec once + res = spec.Eval(specCtx, t) + if res.HasRuntimeError() { + rterr = res.RuntimeError() + t.Fatal(rterr) + } + debug.Println( + ctx, "run: single-shot (no retries) ok: %v", + !res.Failed(), + ) + } else { + // retry the action and test the assertions until they succeed, + // there is a terminal failure, or the timeout expires. + var bo backoff.BackOff + + if rt.Exponential { + bo = backoff.WithContext( + backoff.NewExponentialBackOff(), + ctx, + ) + } else { + interval := gdttypes.DefaultRetryConstantInterval + if rt.Interval != "" { + interval = rt.IntervalDuration() + } + bo = backoff.WithContext( + backoff.NewConstantBackOff(interval), + ctx, + ) + } + ticker := backoff.NewTicker(bo) + maxAttempts := 0 + if rt.Attempts != nil { + maxAttempts = *rt.Attempts + } + attempts := 1 + start := time.Now().UTC() + success := false + for tick := range ticker.C { + if (maxAttempts > 0) && (attempts > maxAttempts) { + debug.Println( + ctx, "run: exceeded max attempts %d. stopping.", + maxAttempts, + ) + ticker.Stop() + break + } + after := tick.Sub(start) + + res = spec.Eval(specCtx, t) + if res.HasRuntimeError() { + rterr = res.RuntimeError() + t.Fatal(rterr) + break + } + success = !res.Failed() + debug.Println( + ctx, "run: attempt %d after %s ok: %v", + attempts, after, success, + ) + if success { + ticker.Stop() + break + } + for _, f := range res.Failures() { + debug.Println( + ctx, "run: attempt %d after %s failure: %s", + attempts, after, f, + ) + } + attempts++ + } + } + for _, fail := range res.Failures() { + t.Error(fail) } // Results can have arbitrary run data stored in them and we // save this prior run data in the top-level context (and pass @@ -107,12 +189,11 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error { return rterr } -// specTimeout returns the timeout value for the test spec. If the spec has a +// getTimeout returns the timeout value for the test spec. If the spec has a // timeout override, we use that. Otherwise, we inspect the scenario's defaults // and, if present, use that timeout. -func specTimeout( +func getTimeout( ctx context.Context, - t *testing.T, specTimeout *gdttypes.Timeout, scenDefaults *Defaults, ) *gdttypes.Timeout { @@ -132,3 +213,54 @@ func specTimeout( } return nil } + +// getRetry returns the retry configuration for the test spec. If the spec has a +// retry override, we use that. Otherwise, we inspect the scenario's defaults +// and, if present, use that timeout. If the scenario's defaults do not +// indicate a retry configuration, we ask the plugin if it has retry defaults +// and use that. +func getRetry( + ctx context.Context, + specRetry *gdttypes.Retry, + scenDefaults *Defaults, + pluginRetry *gdttypes.Retry, +) *gdttypes.Retry { + if specRetry != nil { + msg := "using retry" + if specRetry.Attempts != nil { + msg += fmt.Sprintf(" (attempts: %d)", *specRetry.Attempts) + } + if specRetry.Interval != "" { + msg += fmt.Sprintf(" (interval: %s)", specRetry.Interval) + } + msg += fmt.Sprintf(" (exponential: %t)", specRetry.Exponential) + debug.Println(ctx, msg) + return specRetry + } + if scenDefaults != nil && scenDefaults.Retry != nil { + scenRetry := scenDefaults.Retry + msg := "using retry" + if scenRetry.Attempts != nil { + msg += fmt.Sprintf(" (attempts: %d)", *scenRetry.Attempts) + } + if scenRetry.Interval != "" { + msg += fmt.Sprintf(" (interval: %s)", scenRetry.Interval) + } + msg += fmt.Sprintf(" (exponential: %t) [scenario default]", scenRetry.Exponential) + debug.Println(ctx, msg) + return scenRetry + } + if pluginRetry != nil { + msg := "using retry" + if pluginRetry.Attempts != nil { + msg += fmt.Sprintf(" (attempts: %d)", *pluginRetry.Attempts) + } + if pluginRetry.Interval != "" { + msg += fmt.Sprintf(" (interval: %s)", pluginRetry.Interval) + } + msg += fmt.Sprintf(" (exponential: %t) [plugin default]", pluginRetry.Exponential) + debug.Println(ctx, msg) + return pluginRetry + } + return nil +} diff --git a/scenario/run_test.go b/scenario/run_test.go index 7d98d23..f2585fa 100644 --- a/scenario/run_test.go +++ b/scenario/run_test.go @@ -8,7 +8,10 @@ import ( "bufio" "bytes" "context" + "flag" + "fmt" "os" + "os/exec" "path/filepath" "testing" @@ -19,6 +22,8 @@ import ( "github.com/stretchr/testify/require" ) +var failFlag = flag.Bool("fail", false, "run tests expected to fail") + func TestRun(t *testing.T) { require := require.New(t) @@ -111,19 +116,64 @@ func TestDebugFlushing(t *testing.T) { require.Contains(debugout, "[gdt] [foo-debug-wait-flush] wait: 250ms before") } -func TestTimeoutCascade(t *testing.T) { +func TestNoRetry(t *testing.T) { require := require.New(t) - fp := filepath.Join("testdata", "foo-timeout.yaml") + fp := filepath.Join("testdata", "no-retry.yaml") f, err := os.Open(fp) require.Nil(err) + var b bytes.Buffer + w := bufio.NewWriter(&b) + ctx := gdtcontext.New(gdtcontext.WithDebug(w)) + s, err := scenario.FromReader(f, scenario.WithPath(fp)) require.Nil(err) require.NotNil(s) - err = s.Run(context.TODO(), t) + err = s.Run(ctx, t) require.Nil(err) + require.False(t.Failed()) + w.Flush() + require.NotEqual(b.Len(), 0) + debugout := b.String() + require.Contains(debugout, "[gdt] [no-retry] run: single-shot (no retries) ok: true") +} + +func TestFailRetryTestOverride(t *testing.T) { + if !*failFlag { + t.Skip("skipping without -fail flag") + } + require := require.New(t) + + fp := filepath.Join("testdata", "retry-test-override.yaml") + f, err := os.Open(fp) + require.Nil(err) + + s, err := scenario.FromReader(f, scenario.WithPath(fp)) + require.Nil(err) + require.NotNil(s) + + ctx := gdtcontext.New(gdtcontext.WithDebug()) + err = s.Run(ctx, t) + require.Nil(err) +} + +func TestRetryTestOverride(t *testing.T) { + require := require.New(t) + target := os.Args[0] + failArgs := []string{ + "-test.v", + "-test.run=FailRetryTestOverride", + "-fail", + } + outerr, err := exec.Command(target, failArgs...).CombinedOutput() + + // The test should have failed... + require.NotNil(err) + + debugout := fmt.Sprintf("%s", outerr) + require.Contains(debugout, "[gdt] [retry-test-override] run: exceeded max attempts 2. stopping.") } func TestSkipIf(t *testing.T) { diff --git a/scenario/scenario.go b/scenario/scenario.go index 8d2a270..18716e2 100644 --- a/scenario/scenario.go +++ b/scenario/scenario.go @@ -13,6 +13,9 @@ import ( // Scenario is a generalized gdt test case file. It contains a set of Runnable // test units. type Scenario struct { + // evalPlugins stores the plugin that will evaluate the test spec at a + // particular index + evalPlugins map[int]gdttypes.Plugin // Path is the filepath to the test case. Path string `yaml:"-"` // Name is the short name for the test case. If empty, defaults to the base diff --git a/scenario/stub_plugins_test.go b/scenario/stub_plugins_test.go index 70287cb..f702bda 100644 --- a/scenario/stub_plugins_test.go +++ b/scenario/stub_plugins_test.go @@ -238,6 +238,9 @@ func (s *fooSpec) Eval(ctx context.Context, t *testing.T) *result.Result { fails = append(fails, fail) } }) + for _, fail := range fails { + t.Error(fail) + } return result.New(result.WithFailures(fails...)) } diff --git a/scenario/testdata/no-retry.yaml b/scenario/testdata/no-retry.yaml new file mode 100644 index 0000000..4628858 --- /dev/null +++ b/scenario/testdata/no-retry.yaml @@ -0,0 +1,6 @@ +name: no-retry +description: a scenario using a plugin with no retries +tests: + # The foo plugin does not use retries + - foo: bar + name: bar diff --git a/scenario/testdata/parse/fail/bad-retry-attempts.yaml b/scenario/testdata/parse/fail/bad-retry-attempts.yaml new file mode 100644 index 0000000..f44b9ab --- /dev/null +++ b/scenario/testdata/parse/fail/bad-retry-attempts.yaml @@ -0,0 +1,6 @@ +name: bad-retry-attempts +description: a scenario with an invalid retry attempts +tests: + - foo: baz + retry: + attempts: -1 diff --git a/scenario/testdata/parse/fail/bad-retry-interval-duration.yaml b/scenario/testdata/parse/fail/bad-retry-interval-duration.yaml new file mode 100644 index 0000000..086c62c --- /dev/null +++ b/scenario/testdata/parse/fail/bad-retry-interval-duration.yaml @@ -0,0 +1,6 @@ +name: bad-retry-interval-duration +description: a scenario with an invalid duration string in a spec retry interval +tests: + - foo: baz + retry: + interval: notaduration diff --git a/scenario/testdata/parse/fail/bad-retry.yaml b/scenario/testdata/parse/fail/bad-retry.yaml new file mode 100644 index 0000000..f68cf13 --- /dev/null +++ b/scenario/testdata/parse/fail/bad-retry.yaml @@ -0,0 +1,5 @@ +name: bad-retry +description: a scenario with an invalid retry spec +tests: + - foo: baz + retry: notaretry diff --git a/scenario/testdata/retry-test-override.yaml b/scenario/testdata/retry-test-override.yaml new file mode 100644 index 0000000..41c6747 --- /dev/null +++ b/scenario/testdata/retry-test-override.yaml @@ -0,0 +1,9 @@ +name: retry-test-override +description: a scenario using a test spec override for a retry +tests: + # The foo plugin fails if foo == bar but name != bar + - foo: bar + name: baz + retry: + attempts: 2 + interval: .25s diff --git a/types/plugin.go b/types/plugin.go index a182d29..ff4043b 100644 --- a/types/plugin.go +++ b/types/plugin.go @@ -15,6 +15,9 @@ type PluginInfo struct { Aliases []string // Description describes what types of tests the plugin can handle. Description string + // Retry is a Retry that should be used by default for test specs of this + // plugin. + Retry *Retry } // Plugin is the driver interface for different types of gdt tests. diff --git a/types/retry.go b/types/retry.go new file mode 100644 index 0000000..d8df33d --- /dev/null +++ b/types/retry.go @@ -0,0 +1,46 @@ +// Use and distribution licensed under the Apache license version 2. +// +// See the COPYING file in the root project directory for full text. + +package types + +import ( + "time" +) + +const ( + // DefaultRetryAttempts indicates the default number of times to retry + // retries when the plugin uses retries but has not specified a number of + // attempts. + DefaultRetryAttempts = 3 * time.Second + // DefaultRetryConstantInterval indicates the default interval to use for + // retries when the plugin uses retries but does not use exponential + // backoff. + DefaultRetryConstantInterval = 3 * time.Second +) + +// Retry contains information about the number of attempts and interval +// duration with which a Plugin should re-run a Spec's action if the Spec's +// assertions fail. +type Retry struct { + // Attempts is the number of times that the test unit should be retried in + // the event of assertion failure. + Attempts *int `yaml:"attempts,omitempty"` + // Interval is the amount of time that the plugin should wait before + // retrying the test unit in the event of assertion failure. + // Specify a duration using Go's time duration string. + // See https://pkg.go.dev/time#ParseDuration + Interval string `yaml:"interval,omitempty"` + // Exponential indicates that an exponential backoff should be applied to + // the retry. When true, the value of Interval, if any, is used as the + // initial interval for the backoff algoritm. + Exponential bool `yaml:"exponential,omitempty"` +} + +// IntervalDuration returns the time duration of the Retry.Interval +func (r *Retry) IntervalDuration() time.Duration { + // Parsing already validated the duration string so no need to check again + // here + dur, _ := time.ParseDuration(r.Interval) + return dur +} diff --git a/types/spec.go b/types/spec.go index 9bc5998..6d090ec 100644 --- a/types/spec.go +++ b/types/spec.go @@ -21,6 +21,7 @@ var ( "description", "timeout", "wait", + "retry", } ) @@ -41,6 +42,8 @@ type Spec struct { Timeout *Timeout `yaml:"timeout,omitempty"` // Wait contains the wait configuration for the Spec Wait *Wait `yaml:"wait,omitempty"` + // Retry contains the retry configuration for the Spec + Retry *Retry `yaml:"retry,omitempty"` } // Title returns the Name of the scenario or the Path's file/base name if there @@ -135,6 +138,27 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { } } s.Wait = w + case "retry": + if valNode.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(valNode) + } + var r *Retry + if err := valNode.Decode(&r); err != nil { + return errors.ExpectedRetryAt(valNode) + } + if r.Attempts != nil { + attempts := *r.Attempts + if attempts < 1 { + return errors.InvalidRetryAttempts(valNode, attempts) + } + } + if r.Interval != "" { + _, err := time.ParseDuration(r.Interval) + if err != nil { + return err + } + } + s.Retry = r } } return nil