Skip to content

Commit

Permalink
check go test -timeout before running scenario
Browse files Browse the repository at this point in the history
Adds code that totals up wait times and max timeout values and checks
that the `go test -timeout` value (which defaults to 10m) is smaller
than either the total wait time or largest timeout and returns a
RuntimeError if so, allowing the user to adjust their `go test -timeout`
value or scenario/spec timeouts/waits.

Issue #45

Signed-off-by: Jay Pipes <[email protected]>
  • Loading branch information
jaypipes committed Jul 6, 2024
1 parent 8548d8c commit 2ccf018
Show file tree
Hide file tree
Showing 12 changed files with 256 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ VERSION ?= $(shell git describe --tags --always --dirty)
.PHONY: test

test:
go test -v ./...
go test -timeout 10s -v ./...
37 changes: 37 additions & 0 deletions api/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package api
import (
"errors"
"fmt"
"time"

"gopkg.in/yaml.v3"
)
Expand Down Expand Up @@ -294,10 +295,46 @@ var (
"%w: required fixture missing",
RuntimeError,
)
// ErrTimeoutConflict is returned when the Go test tool's timeout conflicts
// with either a total wait time or a timeout in a scenario or test spec
ErrTimeoutConflict = fmt.Errorf(
"%w: timeout conflict",
RuntimeError,
)
)

// RequiredFixtureMissing returns an ErrRequiredFixture with the supplied
// fixture name
func RequiredFixtureMissing(name string) error {
return fmt.Errorf("%w: %s", ErrRequiredFixture, name)
}

// TimeoutConflict returns an ErrTimeoutConflict describing how the Go test
// tool's timeout conflicts with either a total wait time or a timeout value
// from a scenario or spec.
func TimeoutConflict(
gotestDeadline time.Duration,
totalWait time.Duration,
maxTimeout time.Duration,
) error {
msg := fmt.Sprintf(
"go test -timeout value of %s ",
(gotestDeadline + time.Second).Round(time.Second),
)
if totalWait > 0 {
msg += fmt.Sprintf(
"is shorter than the total wait time in the scenario: %s. "+
"either decrease the wait times or increase the "+
"go test -timeout value.",
totalWait.Round(time.Second),
)
} else {
msg += fmt.Sprintf(
"is shorter than the maximum timeout specified in the "+
"scenario: %s. either decrease the scenario or spec "+
"timeout or increase the go test -timeout value.",
maxTimeout.Round(time.Second),
)
}
return fmt.Errorf("%w: %s", ErrTimeoutConflict, msg)
}
71 changes: 71 additions & 0 deletions api/timings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Use and distribution licensed under the Apache license version 2.
//
// See the COPYING file in the root project directory for full text.

package api

import (
"time"
)

type SetOn int

const (
SetOnNone SetOn = iota
SetOnSpec // a test spec override
SetOnPlugin // a plugin override
SetOnPluginDefault // a plugin default
SetOnDefault // a scenario default
)

// Timings contains information about a test scenario's maximum wait and
// timeout duration and what aspect of the scenario (the scenario defaults, a
// plugin default, a test spec override, etc) had the maximum timeout or wait
// value.
//
// We use this information when initially assessing whether the Go test tool's
// overall timeout is shorter than this maximum in order to inform the user to
// increase the Go test tool timeout.
type Timings struct {
// GoTestTimeout will be the duration of the timeout specified (or
// defaulted) by the Go test tool
GoTestTimeout time.Duration
// TotalWait will be non-zero when there is a wait specified for either the
// scenario or a test spec and will contain the aggregate duration of all
// waits
TotalWait time.Duration
// MaxTimeout will be non-zero when there is a timeout specified for either
// the scenario or a test spec and will contain the duration of the maximum
// timeout
MaxTimeout time.Duration
//TimeoutSetOn indicates where the MaxTimeout value was found
MaxTimeoutSetOn SetOn
// TimeoutSpecIndex indicates the test spec's index within the scenario where
// the max timeout was found
MaxTimeoutSpecIndex int
}

// AddWait adds a wait duration to the Timings and (re)-calculates the Timings'
// MaxWait attributes
func (t *Timings) AddWait(
d time.Duration,
) {
t.TotalWait += d
}

// AddTimeout adds a timeout duration to the Timings and (re)-calculates the
// Timings' MaxTimeout attributes
func (t *Timings) AddTimeout(
d time.Duration,
on SetOn,
specIndex int,
) {
if d == 0 {
return
}
if d.Abs() > t.MaxTimeout {
t.MaxTimeout = d
t.MaxTimeoutSetOn = on
t.MaxTimeoutSpecIndex = specIndex
}
}
18 changes: 13 additions & 5 deletions scenario/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,20 @@ func (d *Defaults) UnmarshalYAML(node *yaml.Node) error {
valNode := node.Content[i+1]
switch key {
case "timeout":
if valNode.Kind != yaml.MappingNode {
return api.ExpectedMapAt(valNode)
}
var to *api.Timeout
if err := valNode.Decode(&to); err != nil {
return api.ExpectedTimeoutAt(valNode)
switch valNode.Kind {
case yaml.MappingNode:
// We support the old-style timeout:after
if err := valNode.Decode(&to); err != nil {
return api.ExpectedTimeoutAt(valNode)
}
case yaml.ScalarNode:
// We also support a straight string duration
to = &api.Timeout{
After: valNode.Value,
}
default:
return api.ExpectedScalarOrMapAt(valNode)
}
_, err := time.ParseDuration(to.After)
if err != nil {
Expand Down
29 changes: 28 additions & 1 deletion scenario/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func (s *Scenario) UnmarshalYAML(node *yaml.Node) error {
if node.Kind != yaml.MappingNode {
return api.ExpectedMapAt(node)
}
s.Timings = &api.Timings{}
plugins := plugin.Registered()
defaults := api.Defaults{}
// maps/structs are stored in a top-level Node.Content field which is a
Expand Down Expand Up @@ -78,6 +79,13 @@ func (s *Scenario) UnmarshalYAML(node *yaml.Node) error {
if err := valNode.Decode(&scenDefaults); err != nil {
return err
}
if scenDefaults.Timeout != nil {
s.Timings.AddTimeout(
scenDefaults.Timeout.Duration(),
api.SetOnDefault,
-1,
)
}
defaults[DefaultsKey] = &scenDefaults
s.Defaults = defaults
}
Expand Down Expand Up @@ -107,14 +115,33 @@ func (s *Scenario) UnmarshalYAML(node *yaml.Node) error {
pluginSpecs[p] = p.Specs()
}
for plugin, specs := range pluginSpecs {
for _, sp := range specs {
for idx, sp := range specs {
if err := testNode.Decode(sp); err != nil {
if errors.Is(err, api.ErrUnknownField) {
continue
}
return err
}
base.Plugin = plugin
if base.Wait != nil {
if base.Wait.Before != "" {
s.Timings.AddWait(
base.Wait.BeforeDuration(),
)
}
if base.Wait.After != "" {
s.Timings.AddWait(
base.Wait.AfterDuration(),
)
}
}
if base.Timeout != nil {
s.Timings.AddTimeout(
base.Timeout.Duration(),
api.SetOnSpec,
idx,
)
}
sp.SetBase(base)
s.Tests = append(s.Tests, sp)
parsed = true
Expand Down
36 changes: 32 additions & 4 deletions scenario/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,38 @@ import (
// which will mark the test units failed or skipped if a test unit evaluates to
// false.
func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
ctx = gdtcontext.PushTrace(ctx, s.Title())
defer func() {
ctx = gdtcontext.PopTrace(ctx)
}()
now := time.Now()
d, ok := t.Deadline()
if ok && !d.IsZero() {
s.Timings.GoTestTimeout = d.Sub(now)
debug.Println(
ctx, "scenario/run: go test tool deadline: %s",
(s.Timings.GoTestTimeout + time.Second).Round(time.Second),
)
if s.Timings.TotalWait > 0 {
if s.Timings.TotalWait.Abs() > s.Timings.GoTestTimeout.Abs() {
return api.TimeoutConflict(
s.Timings.GoTestTimeout,
s.Timings.TotalWait,
s.Timings.MaxTimeout,
)
}
}
if s.Timings.MaxTimeout > 0 {
if s.Timings.MaxTimeout.Abs() > s.Timings.GoTestTimeout.Abs() {
return api.TimeoutConflict(
s.Timings.GoTestTimeout,
s.Timings.TotalWait,
s.Timings.MaxTimeout,
)
}
}
}

if len(s.Fixtures) > 0 {
fixtures := gdtcontext.Fixtures(ctx)
for _, fname := range s.Fixtures {
Expand Down Expand Up @@ -62,10 +94,6 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
scDefaults := s.getScenarioDefaults()

t.Run(s.Title(), func(t *testing.T) {
ctx = gdtcontext.PushTrace(ctx, s.Title())
defer func() {
ctx = gdtcontext.PopTrace(ctx)
}()
for idx, spec := range s.Tests {
sb := spec.Base()

Expand Down
54 changes: 54 additions & 0 deletions scenario/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,60 @@ func TestMissingFixtures(t *testing.T) {
assert.ErrorIs(err, api.RuntimeError)
}

func TestTimeoutConflictTotalWait(t *testing.T) {
require := require.New(t)
assert := assert.New(t)

fp := filepath.Join("testdata", "timeout-conflict-total-wait.yaml")
f, err := os.Open(fp)
require.Nil(err)

s, err := scenario.FromReader(f, scenario.WithPath(fp))
require.Nil(err)
require.NotNil(s)

err = s.Run(context.TODO(), t)
assert.NotNil(err)
assert.ErrorIs(err, api.ErrTimeoutConflict)
assert.ErrorIs(err, api.RuntimeError)
}

func TestTimeoutConflictSpecTimeout(t *testing.T) {
require := require.New(t)
assert := assert.New(t)

fp := filepath.Join("testdata", "timeout-conflict-spec-timeout.yaml")
f, err := os.Open(fp)
require.Nil(err)

s, err := scenario.FromReader(f, scenario.WithPath(fp))
require.Nil(err)
require.NotNil(s)

err = s.Run(context.TODO(), t)
assert.NotNil(err)
assert.ErrorIs(err, api.ErrTimeoutConflict)
assert.ErrorIs(err, api.RuntimeError)
}

func TestTimeoutConflictDefaultTimeout(t *testing.T) {
require := require.New(t)
assert := assert.New(t)

fp := filepath.Join("testdata", "timeout-conflict-default-timeout.yaml")
f, err := os.Open(fp)
require.Nil(err)

s, err := scenario.FromReader(f, scenario.WithPath(fp))
require.Nil(err)
require.NotNil(s)

err = s.Run(context.TODO(), t)
assert.NotNil(err)
assert.ErrorIs(err, api.ErrTimeoutConflict)
assert.ErrorIs(err, api.RuntimeError)
}

func TestFixtureStartError(t *testing.T) {
require := require.New(t)
assert := assert.New(t)
Expand Down
7 changes: 3 additions & 4 deletions scenario/scenario.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +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]api.Plugin
// Path is the filepath to the test case.
// Timings is the collection of max wait/timeout values for the scenario.
Timings *api.Timings `yaml:"-"`
// Path is the filepath to the test scenario YAML file.
Path string `yaml:"-"`
// Name is the short name for the test case. If empty, defaults to the base
// filename in Path.
Expand Down
1 change: 0 additions & 1 deletion scenario/testdata/fixture-start-error.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@ fixtures:
- start-error
tests:
- foo: bar

6 changes: 6 additions & 0 deletions scenario/testdata/timeout-conflict-default-timeout.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name: timeout-conflict-default-timeout
description: a scenario with a default timeout greater than the go test tool timeout
defaults:
timeout: 1m
tests:
- foo: bar
5 changes: 5 additions & 0 deletions scenario/testdata/timeout-conflict-spec-timeout.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
name: timeout-conflict-spec-timeout
description: a scenario with a spec timeout greater than the go test tool timeout
tests:
- timeout: 1m
foo: bar
6 changes: 6 additions & 0 deletions scenario/testdata/timeout-conflict-total-wait.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name: timeout-conflict-total-wait
description: a scenario with total wait times greater than the go test tool timeout
tests:
- wait:
before: 1m
foo: bar

0 comments on commit 2ccf018

Please sign in to comment.