Merge pull request #32 from jaypipes/timeout-plugin-specified

allow plugin-defaulted timeout values

Author: jaypipes

README.md

@@ -458,12 +458,8 @@ All test specs have the following fields:
 
 * `name`: (optional) string describing the test unit.
 * `description`: (optional) string with longer description of the test unit.
-* `timeout`: (optional) an object containing [timeout information][timeout] for the test
-  unit.
-* `timeout.after`: a string duration of time the test unit is expected to
+* `timeout`: (optional) a string duration of time the test unit is expected to
   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
@@ -497,7 +493,6 @@ All test specs have the following fields:
 [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

plugin/exec/eval_test.go

@@ -16,10 +16,15 @@ import (
 	"testing"
 
 	gdtcontext "github.com/gdt-dev/gdt/context"
+	execplugin "github.com/gdt-dev/gdt/plugin/exec"
 	"github.com/gdt-dev/gdt/scenario"
 	"github.com/stretchr/testify/require"
 )
 
+func init() {
+	execplugin.OverrideDefaultTimeout("0.5s")
+}
+
 var failFlag = flag.Bool("fail", false, "run tests expected to fail")
 
 func TestNoExitCodeSimpleCommand(t *testing.T) {
@@ -161,6 +166,45 @@ func TestContainsNoneOf(t *testing.T) {
 	require.Nil(err)
 }
 
+func TestFailExecTimeoutPluginDefault(t *testing.T) {
+	if !*failFlag {
+		t.Skip("skipping without -fail flag")
+	}
+	require := require.New(t)
+
+	fp := filepath.Join("testdata", "timeout-plugin-default.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 TestExecTimeoutPluginDefault(t *testing.T) {
+	require := require.New(t)
+	target := os.Args[0]
+	failArgs := []string{
+		"-test.v",
+		"-test.run=FailExecTimeoutPluginDefault",
+		"-fail",
+	}
+	outerr, err := exec.Command(target, failArgs...).CombinedOutput()
+
+	// The test should have failed...
+	require.NotNil(err)
+	debugout := string(outerr)
+	require.Contains(debugout, "using timeout of 0.5s [plugin default]")
+	require.Contains(debugout, "assertion failed: timeout exceeded")
+}
+
 func TestFailExecSleepTimeout(t *testing.T) {
 	if !*failFlag {
 		t.Skip("skipping without -fail flag")
@@ -289,8 +333,8 @@ func TestExecTimeoutCascade(t *testing.T) {
 	require.NotNil(err)
 
 	debugout := string(outerr)
-	require.Contains(debugout, "using timeout of 500ms (expected: false) [scenario default]")
-	require.Contains(debugout, "using timeout of 20ms (expected: true)")
+	require.Contains(debugout, "using timeout of 500ms [scenario default]")
+	require.Contains(debugout, "using timeout of 20ms")
 }
 
 func TestFailExecOnFail(t *testing.T) {

plugin/exec/plugin.go

@@ -11,6 +11,15 @@ import (
 	gdttypes "github.com/gdt-dev/gdt/types"
 )
 
+var (
+	DefaultTimeout = "10s"
+)
+
+// OverrideDefaultTimeout is only used in testing...
+func OverrideDefaultTimeout(d string) {
+	DefaultTimeout = d
+}
+
 func init() {
 	gdtplugin.Register(Plugin())
 }
@@ -24,6 +33,9 @@ type plugin struct{}
 func (p *plugin) Info() gdttypes.PluginInfo {
 	return gdttypes.PluginInfo{
 		Name: pluginName,
+		Timeout: &gdttypes.Timeout{
+			After: DefaultTimeout,
+		},
 	}
 }
 

plugin/exec/testdata/sleep-timeout.yaml

@@ -7,4 +7,3 @@ tests:
   - exec: sleep 5
     timeout:
       after: 50ms
-      expected: true

plugin/exec/testdata/timeout-cascade.yaml

@@ -10,4 +10,3 @@ tests:
     exec: sleep .25
     timeout:
       after: 20ms
-      expected: true

plugin/exec/testdata/timeout-plugin-default.yaml

@@ -0,0 +1,5 @@
+name: timeout-plugin-default
+description: a scenario that tests the plugin default timeout
+tests:
+  - name: uses plugin default timeout
+    exec: sleep 1

scenario/parse_test.go

@@ -92,6 +92,18 @@ func TestUnknownSpec(t *testing.T) {
 	assert.Nil(s)
 }
 
+func TestTimeoutScalarOrMap(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	fp := filepath.Join("testdata", "parse", "timeout-scalar-or-map.yaml")
+	f, err := os.Open(fp)
+	require.Nil(err)
+
+	_, err = scenario.FromReader(f, scenario.WithPath(fp))
+	assert.Nil(err)
+}
+
 func TestBadTimeout(t *testing.T) {
 	assert := assert.New(t)
 	require := require.New(t)
@@ -101,7 +113,7 @@ func TestBadTimeout(t *testing.T) {
 	require.Nil(err)
 
 	s, err := scenario.FromReader(f, scenario.WithPath(fp))
-	assert.ErrorIs(err, errors.ErrExpectedMap)
+	assert.ErrorIs(err, errors.ErrExpectedScalarOrMap)
 	assert.Nil(s)
 }
 

scenario/run.go

@@ -73,6 +73,7 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 			plugin := s.evalPlugins[idx]
 			pinfo := plugin.Info()
 			pretry := pinfo.Retry
+			ptimeout := pinfo.Timeout
 
 			// 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
@@ -88,7 +89,7 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 				time.Sleep(wait.BeforeDuration())
 			}
 
-			to := getTimeout(ctx, sb.Timeout, scDefaults)
+			to := getTimeout(ctx, sb.Timeout, ptimeout, scDefaults)
 			if to != nil {
 				var cancel context.CancelFunc
 				specCtx, cancel = context.WithTimeout(specCtx, to.Duration())
@@ -191,26 +192,36 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 
 // 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.
+// and, if present, use that timeout. If the scenario's defaults for not
+// indicate a timeout configuration, we ask the plugin if it has timeout
+// defaults and use that.
 func getTimeout(
 	ctx context.Context,
 	specTimeout *gdttypes.Timeout,
+	pluginTimeout *gdttypes.Timeout,
 	scenDefaults *Defaults,
 ) *gdttypes.Timeout {
 	if specTimeout != nil {
 		debug.Println(
-			ctx, "using timeout of %s (expected: %t)",
-			specTimeout.After, specTimeout.Expected,
+			ctx, "using timeout of %s",
+			specTimeout.After,
 		)
 		return specTimeout
 	}
 	if scenDefaults != nil && scenDefaults.Timeout != nil {
 		debug.Println(
-			ctx, "using timeout of %s (expected: %t) [scenario default]",
-			scenDefaults.Timeout.After, scenDefaults.Timeout.Expected,
+			ctx, "using timeout of %s [scenario default]",
+			scenDefaults.Timeout.After,
 		)
 		return scenDefaults.Timeout
 	}
+	if pluginTimeout != nil {
+		debug.Println(
+			ctx, "using timeout of %s [plugin default]",
+			pluginTimeout.After,
+		)
+		return pluginTimeout
+	}
 	return nil
 }
 

scenario/testdata/parse/fail/bad-timeout.yaml

@@ -2,4 +2,5 @@ name: bad-timeout
 description: a scenario with an invalid timeout spec
 tests:
   - foo: baz
-    timeout: notatimeout
+    timeout:
+      - one

scenario/testdata/parse/timeout-scalar-or-map.yaml

@@ -0,0 +1,8 @@
+name: timeout-scalar-or-map
+description: a scenario with both scalar and object timeout spec
+tests:
+  - foo: baz
+    timeout: 1s
+  - foo: bar
+    timeout:
+      after: 1s

types/plugin.go

@@ -15,6 +15,9 @@ type PluginInfo struct {
 	Aliases []string
 	// Description describes what types of tests the plugin can handle.
 	Description string
+	// Timeout is a Timeout that should be used by default for test specs of
+	// this plugin.
+	Timeout *Timeout
 	// Retry is a Retry that should be used by default for test specs of this
 	// plugin.
 	Retry *Retry

types/spec.go

@@ -105,12 +105,20 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error {
 			}
 			s.Description = valNode.Value
 		case "timeout":
-			if valNode.Kind != yaml.MappingNode {
-				return errors.ExpectedMapAt(valNode)
-			}
 			var to *Timeout
-			if err := valNode.Decode(&to); err != nil {
-				return errors.ExpectedTimeoutAt(valNode)
+			switch valNode.Kind {
+			case yaml.MappingNode:
+				// We support the old-style timeout:after
+				if err := valNode.Decode(&to); err != nil {
+					return errors.ExpectedTimeoutAt(valNode)
+				}
+			case yaml.ScalarNode:
+				// We also support a straight string duration
+				to = &Timeout{
+					After: valNode.Value,
+				}
+			default:
+				return errors.ExpectedScalarOrMapAt(valNode)
 			}
 			_, err := time.ParseDuration(to.After)
 			if err != nil {

types/timeout.go

@@ -16,9 +16,6 @@ type Timeout struct {
 	// Specify a duration using Go's time duration string.
 	// See https://pkg.go.dev/time#ParseDuration
 	After string `yaml:"after,omitempty"`
-	// Expected indicates whether the timeout is expected to be exceeded. This
-	// is mostly useful for unit testing of the timeout functionality itself.
-	Expected bool `yaml:"expected,omitempty"`
 }
 
 // Duration returns the time duration of the Timeout