Merge pull request #46 from jaypipes/issue45

check go test -timeout before running scenario

.github/workflows/test.yml

@@ -36,4 +36,4 @@ jobs:
        with:
          go-version: ${{ matrix.go }}
          check-latest: true
-     - run: go test -v ./...
+     - run: go test -timeout 10s -v ./...

Makefile

@@ -3,4 +3,4 @@ VERSION ?= $(shell git describe --tags --always --dirty)
 .PHONY: test
 
 test:
-	go test -v ./...
+	go test -timeout 10s -v ./...

api/error.go

@@ -7,6 +7,7 @@ package api
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	"gopkg.in/yaml.v3"
 )
@@ -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)
+}

api/timings.go

@@ -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
+	}
+}

scenario/defaults.go

@@ -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 {

scenario/parse.go

@@ -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
@@ -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
 		}
@@ -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

scenario/run.go

@@ -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 {
@@ -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()
 

scenario/run_test.go

@@ -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)

scenario/scenario.go

@@ -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.

scenario/testdata/fixture-start-error.yaml

@@ -4,4 +4,3 @@ fixtures:
   - start-error
 tests:
   - foo: bar
-

scenario/testdata/timeout-conflict-default-timeout.yaml

@@ -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: baz

scenario/testdata/timeout-conflict-spec-timeout.yaml

@@ -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: baz

scenario/testdata/timeout-conflict-total-wait.yaml

@@ -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: baz