allow an Evaluable to override retry behaviour

We need to allow a plugin's Evaluable to override the plugin's default
retry behaviour. In the case of gdt-kube, we want to *not* retry in the
case of `kubectl.create`, `kubectl.apply` and `kubectl.delete`, however
we *do* want to retry in the case of `kubectl.get`...

Issue gdt-dev/kube#17

Signed-off-by: Jay Pipes <[email protected]>

context/context_test.go

@@ -39,6 +39,14 @@ func (s *fooSpec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
 
+func (s *fooSpec) Retry() *gdttypes.Retry {
+	return nil
+}
+
+func (s *fooSpec) Timeout() *gdttypes.Timeout {
+	return nil
+}
+
 func (s *fooSpec) UnmarshalYAML(node *yaml.Node) error {
 	return nil
 }

plugin/exec/spec.go

@@ -26,3 +26,11 @@ func (s *Spec) SetBase(b gdttypes.Spec) {
 func (s *Spec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
+
+func (s *Spec) Retry() *gdttypes.Retry {
+	return nil
+}
+
+func (s *Spec) Timeout() *gdttypes.Timeout {
+	return nil
+}

plugin/registry_test.go

@@ -36,6 +36,14 @@ func (s *fooSpec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
 
+func (s *fooSpec) Retry() *gdttypes.Retry {
+	return nil
+}
+
+func (s *fooSpec) Timeout() *gdttypes.Timeout {
+	return nil
+}
+
 func (s *fooSpec) Eval(context.Context) (*result.Result, error) {
 	return nil, nil
 }

scenario/run.go

@@ -78,11 +78,8 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 				time.Sleep(wait.BeforeDuration())
 			}
 			plugin := s.evalPlugins[idx]
-			pinfo := plugin.Info()
-			pretry := pinfo.Retry
-			ptimeout := pinfo.Timeout
 
-			rt := getRetry(ctx, sb.Retry, scDefaults, pretry)
+			rt := getRetry(ctx, scDefaults, plugin, spec)
 
 			// 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
@@ -91,7 +88,7 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 			specCtx, specCancel := context.WithCancel(ctx)
 			defer specCancel()
 
-			to := getTimeout(ctx, sb.Timeout, ptimeout, scDefaults)
+			to := getTimeout(ctx, scDefaults, plugin, spec)
 			if to != nil {
 				var cancel context.CancelFunc
 				specCtx, cancel = context.WithTimeout(specCtx, to.Duration())
@@ -113,7 +110,7 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 				ctx = gdtcontext.StorePriorRun(ctx, res.Data())
 			}
 			for _, fail := range res.Failures() {
-				t.Error(fail)
+				t.Fatal(fail)
 			}
 		}
 	})
@@ -137,7 +134,7 @@ func (s *Scenario) runSpec(
 	defer func() {
 		ctx = gdtcontext.PopTrace(ctx)
 	}()
-	if retry == nil {
+	if retry == nil || retry == gdttypes.NoRetry {
 		// Just evaluate the test spec once
 		res, err := spec.Eval(ctx)
 		if err != nil {
@@ -214,31 +211,49 @@ func (s *Scenario) runSpec(
 	return res, nil
 }
 
-// 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. If the scenario's defaults for not
-// indicate a timeout configuration, we ask the plugin if it has timeout
-// defaults and use that.
+// getTimeout returns the timeout configuration for the test spec. We check for
+// overrides in timeout configuration using the following precedence:
+//
+// * Spec (Evaluable) override
+// * Spec's Base override
+// * Scenario's default
+// * Plugin's default
 func getTimeout(
 	ctx context.Context,
-	specTimeout *gdttypes.Timeout,
-	pluginTimeout *gdttypes.Timeout,
 	scenDefaults *Defaults,
+	plugin gdttypes.Plugin,
+	eval gdttypes.Evaluable,
 ) *gdttypes.Timeout {
-	if specTimeout != nil {
+	evalTimeout := eval.Timeout()
+	if evalTimeout != nil {
+		debug.Println(
+			ctx, "using timeout of %s",
+			evalTimeout.After,
+		)
+		return evalTimeout
+	}
+
+	sb := eval.Base()
+	baseTimeout := sb.Timeout
+	if baseTimeout != nil {
 		debug.Println(
 			ctx, "using timeout of %s",
-			specTimeout.After,
+			baseTimeout.After,
 		)
-		return specTimeout
+		return baseTimeout
 	}
+
 	if scenDefaults != nil && scenDefaults.Timeout != nil {
 		debug.Println(
 			ctx, "using timeout of %s [scenario default]",
 			scenDefaults.Timeout.After,
 		)
 		return scenDefaults.Timeout
 	}
+
+	pluginInfo := plugin.Info()
+	pluginTimeout := pluginInfo.Timeout
+
 	if pluginTimeout != nil {
 		debug.Println(
 			ctx, "using timeout of %s [plugin default]",
@@ -249,31 +264,59 @@ func getTimeout(
 	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.
+// getRetry returns the retry configuration for the test spec. We check for
+// overrides in retry configuration using the following precedence:
+//
+// * Spec (Evaluable) override
+// * Spec's Base override
+// * Scenario's default
+// * Plugin's default
 func getRetry(
 	ctx context.Context,
-	specRetry *gdttypes.Retry,
 	scenDefaults *Defaults,
-	pluginRetry *gdttypes.Retry,
+	plugin gdttypes.Plugin,
+	eval gdttypes.Evaluable,
 ) *gdttypes.Retry {
-	if specRetry != nil {
+	evalRetry := eval.Retry()
+	if evalRetry != nil {
+		if evalRetry == gdttypes.NoRetry {
+			return evalRetry
+		}
 		msg := "using retry"
-		if specRetry.Attempts != nil {
-			msg += fmt.Sprintf(" (attempts: %d)", *specRetry.Attempts)
+		if evalRetry.Attempts != nil {
+			msg += fmt.Sprintf(" (attempts: %d)", *evalRetry.Attempts)
 		}
-		if specRetry.Interval != "" {
-			msg += fmt.Sprintf(" (interval: %s)", specRetry.Interval)
+		if evalRetry.Interval != "" {
+			msg += fmt.Sprintf(" (interval: %s)", evalRetry.Interval)
 		}
-		msg += fmt.Sprintf(" (exponential: %t)", specRetry.Exponential)
+		msg += fmt.Sprintf(" (exponential: %t)", evalRetry.Exponential)
 		debug.Println(ctx, msg)
-		return specRetry
+		return evalRetry
 	}
+
+	sb := eval.Base()
+	baseRetry := sb.Retry
+	if baseRetry != nil {
+		if baseRetry == gdttypes.NoRetry {
+			return baseRetry
+		}
+		msg := "using retry"
+		if baseRetry.Attempts != nil {
+			msg += fmt.Sprintf(" (attempts: %d)", *baseRetry.Attempts)
+		}
+		if baseRetry.Interval != "" {
+			msg += fmt.Sprintf(" (interval: %s)", baseRetry.Interval)
+		}
+		msg += fmt.Sprintf(" (exponential: %t)", baseRetry.Exponential)
+		debug.Println(ctx, msg)
+		return baseRetry
+	}
+
 	if scenDefaults != nil && scenDefaults.Retry != nil {
 		scenRetry := scenDefaults.Retry
+		if scenRetry == gdttypes.NoRetry {
+			return scenRetry
+		}
 		msg := "using retry"
 		if scenRetry.Attempts != nil {
 			msg += fmt.Sprintf(" (attempts: %d)", *scenRetry.Attempts)
@@ -285,7 +328,14 @@ func getRetry(
 		debug.Println(ctx, msg)
 		return scenRetry
 	}
+
+	pluginInfo := plugin.Info()
+	pluginRetry := pluginInfo.Retry
+
 	if pluginRetry != nil {
+		if pluginRetry == gdttypes.NoRetry {
+			return pluginRetry
+		}
 		msg := "using retry"
 		if pluginRetry.Attempts != nil {
 			msg += fmt.Sprintf(" (attempts: %d)", *pluginRetry.Attempts)

scenario/run_test.go

@@ -140,6 +140,30 @@ func TestNoRetry(t *testing.T) {
 	require.Contains(debugout, "[gdt] [no-retry/0:bar] run: single-shot (no retries) ok: true")
 }
 
+func TestNoRetryEvaluableOverride(t *testing.T) {
+	require := require.New(t)
+
+	fp := filepath.Join("testdata", "no-retry-evaluable-override.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(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-evaluable-override/0:bar] run: single-shot (no retries) ok: true")
+}
+
 func TestFailRetryTestOverride(t *testing.T) {
 	if !*failFlag {
 		t.Skip("skipping without -fail flag")

scenario/stub_plugins_test.go

@@ -82,6 +82,14 @@ func (s *failSpec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
 
+func (s *failSpec) Retry() *gdttypes.Retry {
+	return nil
+}
+
+func (s *failSpec) Timeout() *gdttypes.Timeout {
+	return nil
+}
+
 func (s *failSpec) Eval(context.Context) (*result.Result, error) {
 	return nil, fmt.Errorf("%w: Indy, bad dates!", gdterrors.RuntimeError)
 }
@@ -185,6 +193,14 @@ func (s *fooSpec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
 
+func (s *fooSpec) Retry() *gdttypes.Retry {
+	return nil
+}
+
+func (s *fooSpec) Timeout() *gdttypes.Timeout {
+	return nil
+}
+
 func (s *fooSpec) UnmarshalYAML(node *yaml.Node) error {
 	if node.Kind != yaml.MappingNode {
 		return errors.ExpectedMapAt(node)
@@ -266,6 +282,14 @@ func (s *barSpec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
 
+func (s *barSpec) Retry() *gdttypes.Retry {
+	return gdttypes.NoRetry
+}
+
+func (s *barSpec) Timeout() *gdttypes.Timeout {
+	return nil
+}
+
 func (s *barSpec) Eval(context.Context) (*result.Result, error) {
 	return result.New(), nil
 }
@@ -341,6 +365,14 @@ func (s *priorRunSpec) Base() *gdttypes.Spec {
 	return &s.Spec
 }
 
+func (s *priorRunSpec) Retry() *gdttypes.Retry {
+	return nil
+}
+
+func (s *priorRunSpec) Timeout() *gdttypes.Timeout {
+	return nil
+}
+
 func (s *priorRunSpec) UnmarshalYAML(node *yaml.Node) error {
 	if node.Kind != yaml.MappingNode {
 		return errors.ExpectedMapAt(node)

scenario/testdata/no-retry-evaluable-override.yaml

@@ -0,0 +1,6 @@
+name: no-retry-evaluable-override
+description: a scenario using a plugin with an evaluable override for no retries
+tests:
+  # The bar plugin's Evaluable has a NoRetry override
+  - bar: 42
+    name: bar

types/evaluable.go

@@ -23,4 +23,8 @@ type Evaluable interface {
 	SetBase(Spec)
 	// Base returns the Evaluable's base Spec
 	Base() *Spec
+	// Retry returns the Evaluable's Retry override, if any
+	Retry() *Retry
+	// Timeout returns the Evaluable's Timeout override, if any
+	Timeout() *Timeout
 }

types/retry.go

@@ -19,6 +19,12 @@ const (
 	DefaultRetryConstantInterval = 3 * time.Second
 )
 
+var (
+	// NoRetry indicates that there should not be any retry attempts. It is
+	// passed from a plugin to indicate a Spec should not be retried.
+	NoRetry = &Retry{}
+)
+
 // 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.