Merge pull request #47 from jaypipes/cleanup

jaypipes · web-flow · commit b2b65f234d3f · 2024-07-08T08:08:03.000-04:00
break run spec and exec spec into separate methods
diff --git a/api/error.go b/api/error.go
@@ -313,10 +313,11 @@ func RequiredFixtureMissing(name string) error {
 // 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,
+	ti *Timings,
 ) error {
+	gotestDeadline := ti.GoTestTimeout
+	totalWait := ti.TotalWait
+	maxTimeout := ti.MaxTimeout
 	msg := fmt.Sprintf(
 		"go test -timeout value of %s ",
 		(gotestDeadline + time.Second).Round(time.Second),
diff --git a/scenario/run.go b/scenario/run.go
@@ -31,32 +31,9 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 	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 s.hasTimeoutConflict(ctx, t) {
+		return api.TimeoutConflict(s.Timings)
 	}
 
 	if len(s.Fixtures) > 0 {
@@ -73,6 +50,7 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 			defer fix.Stop(ctx)
 		}
 	}
+
 	// If the test author has specified any pre-flight checks in the `skip-if`
 	// collection, evaluate those first and if any failed, skip the scenario's
 	// tests.
@@ -90,79 +68,25 @@ func (s *Scenario) Run(ctx context.Context, t *testing.T) error {
 		}
 	}
 
+	var res *api.Result
 	var err error
-	scDefaults := s.getScenarioDefaults()
-
-	t.Run(s.Title(), func(t *testing.T) {
-		for idx, spec := range s.Tests {
-			sb := spec.Base()
 
-			// 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
-			// first deadline/timeout will be used.
-			specCtx, specCancel := context.WithCancel(ctx)
-
-			specTraceMsg := strconv.Itoa(idx)
-			if sb.Name != "" {
-				specTraceMsg += ":" + sb.Name
-			}
-			specCtx = gdtcontext.PushTrace(specCtx, specTraceMsg)
-			popTracer := func() {
-				specCtx = gdtcontext.PopTrace(specCtx)
-			}
-
-			plugin := sb.Plugin
-
-			rt := getRetry(specCtx, scDefaults, plugin, spec)
-
-			to := getTimeout(specCtx, scDefaults, plugin, spec)
-
-			var res *api.Result
-			ch := make(chan runSpecRes, 1)
-
-			wait := sb.Wait
-			if wait != nil && wait.Before != "" {
-				debug.Println(specCtx, "wait: %s before", wait.Before)
-				time.Sleep(wait.BeforeDuration())
-			}
-
-			if to != nil {
-				specCtx, specCancel = context.WithTimeout(specCtx, to.Duration())
-			}
-
-			go s.runSpec(specCtx, ch, rt, idx, spec)
-
-			select {
-			case <-specCtx.Done():
-				popTracer()
-				t.Fatalf("assertion failed: timeout exceeded (%s)", to.After)
-				break
-			case runres := <-ch:
-				res = runres.r
-				err = runres.err
-			}
+	t.Run(s.Title(), func(tt *testing.T) {
+		for idx := range s.Tests {
+			res, err = s.runSpec(ctx, tt, idx)
 			if err != nil {
-				popTracer()
-				specCancel()
 				break
 			}
 
-			if wait != nil && wait.After != "" {
-				debug.Println(specCtx, "wait: %s after", wait.After)
-				time.Sleep(wait.AfterDuration())
-			}
-
 			// Results can have arbitrary run data stored in them and we
 			// save this prior run data in the top-level context (and pass
 			// that context to the next Run invocation).
 			if res.HasData() {
 				ctx = gdtcontext.StorePriorRun(ctx, res.Data())
 			}
-			popTracer()
-			specCancel()
+
 			for _, fail := range res.Failures() {
-				t.Fatal(fail)
+				tt.Fatal(fail)
 			}
 		}
 	})
@@ -174,8 +98,71 @@ type runSpecRes struct {
 	err error
 }
 
-// runSpec executes an individual test spec
+// runSpec wraps the execution of a single test spec
 func (s *Scenario) runSpec(
+	ctx context.Context, // this is the overall scenario's context
+	t *testing.T, // T specific to the goroutine running this test spec
+	idx int, // index of the test spec within Scenario.Tests
+) (res *api.Result, err error) {
+	// 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
+	// first deadline/timeout will be used.
+	specCtx, specCancel := context.WithCancel(ctx)
+	defer specCancel()
+
+	defaults := s.getDefaults()
+	spec := s.Tests[idx]
+	sb := spec.Base()
+
+	specTraceMsg := strconv.Itoa(idx)
+	if sb.Name != "" {
+		specTraceMsg += ":" + sb.Name
+	}
+	specCtx = gdtcontext.PushTrace(specCtx, specTraceMsg)
+	defer func() {
+		specCtx = gdtcontext.PopTrace(specCtx)
+	}()
+
+	plugin := sb.Plugin
+	rt := getRetry(specCtx, defaults, plugin, spec)
+	to := getTimeout(specCtx, defaults, plugin, spec)
+	ch := make(chan runSpecRes, 1)
+
+	wait := sb.Wait
+	if wait != nil && wait.Before != "" {
+		debug.Println(specCtx, "wait: %s before", wait.Before)
+		time.Sleep(wait.BeforeDuration())
+	}
+
+	if to != nil {
+		specCtx, specCancel = context.WithTimeout(specCtx, to.Duration())
+		defer specCancel()
+	}
+
+	go s.execSpec(specCtx, ch, rt, idx, spec)
+
+	select {
+	case <-specCtx.Done():
+		t.Fatalf("assertion failed: timeout exceeded (%s)", to.After)
+	case runres := <-ch:
+		res = runres.r
+		err = runres.err
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	if wait != nil && wait.After != "" {
+		debug.Println(specCtx, "wait: %s after", wait.After)
+		time.Sleep(wait.AfterDuration())
+	}
+	return res, nil
+}
+
+// execSpec executes an individual test spec, performing any retries as
+// necessary until a timeout is exceeded or the test spec succeeds
+func (s *Scenario) execSpec(
 	ctx context.Context,
 	ch chan runSpecRes,
 	retry *api.Retry,
@@ -190,7 +177,7 @@ func (s *Scenario) runSpec(
 			return
 		}
 		debug.Println(
-			ctx, "run: single-shot (no retries) ok: %v",
+			ctx, "spec/run: single-shot (no retries) ok: %v",
 			!res.Failed(),
 		)
 		ch <- runSpecRes{res, nil}
@@ -229,7 +216,7 @@ func (s *Scenario) runSpec(
 	for tick := range ticker.C {
 		if (maxAttempts > 0) && (attempts > maxAttempts) {
 			debug.Println(
-				ctx, "run: exceeded max attempts %d. stopping.",
+				ctx, "spec/run: exceeded max attempts %d. stopping.",
 				maxAttempts,
 			)
 			ticker.Stop()
@@ -244,7 +231,7 @@ func (s *Scenario) runSpec(
 		}
 		success = !res.Failed()
 		debug.Println(
-			ctx, "run: attempt %d after %s ok: %v",
+			ctx, "spec/run: attempt %d after %s ok: %v",
 			attempts, after, success,
 		)
 		if success {
@@ -253,7 +240,7 @@ func (s *Scenario) runSpec(
 		}
 		for _, f := range res.Failures() {
 			debug.Println(
-				ctx, "run: attempt %d failure: %s",
+				ctx, "spec/run: attempt %d failure: %s",
 				attempts, f,
 			)
 		}
@@ -262,6 +249,34 @@ func (s *Scenario) runSpec(
 	ch <- runSpecRes{res, nil}
 }
 
+// hasTimeoutConflict returns true if the scenario or any of its test specs has
+// a wait or timeout that exceeds the go test tool's specified timeout value
+func (s *Scenario) hasTimeoutConflict(
+	ctx context.Context,
+	t *testing.T,
+) bool {
+	d, ok := t.Deadline()
+	if ok && !d.IsZero() {
+		now := time.Now()
+		s.Timings.GoTestTimeout = d.Sub(now)
+		debug.Println(
+			ctx, "scenario/run: go test tool timeout: %s",
+			(s.Timings.GoTestTimeout + time.Second).Round(time.Second),
+		)
+		if s.Timings.TotalWait > 0 {
+			if s.Timings.TotalWait.Abs() > s.Timings.GoTestTimeout.Abs() {
+				return true
+			}
+		}
+		if s.Timings.MaxTimeout > 0 {
+			if s.Timings.MaxTimeout.Abs() > s.Timings.GoTestTimeout.Abs() {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // getTimeout returns the timeout configuration for the test spec. We check for
 // overrides in timeout configuration using the following precedence:
 //
@@ -271,7 +286,7 @@ func (s *Scenario) runSpec(
 // * Plugin's default
 func getTimeout(
 	ctx context.Context,
-	scenDefaults *Defaults,
+	defaults *Defaults,
 	plugin api.Plugin,
 	eval api.Evaluable,
 ) *api.Timeout {
@@ -294,12 +309,12 @@ func getTimeout(
 		return baseTimeout
 	}
 
-	if scenDefaults != nil && scenDefaults.Timeout != nil {
+	if defaults != nil && defaults.Timeout != nil {
 		debug.Println(
 			ctx, "using timeout of %s [scenario default]",
-			scenDefaults.Timeout.After,
+			defaults.Timeout.After,
 		)
-		return scenDefaults.Timeout
+		return defaults.Timeout
 	}
 
 	pluginInfo := plugin.Info()
@@ -324,7 +339,7 @@ func getTimeout(
 // * Plugin's default
 func getRetry(
 	ctx context.Context,
-	scenDefaults *Defaults,
+	defaults *Defaults,
 	plugin api.Plugin,
 	eval api.Evaluable,
 ) *api.Retry {
@@ -363,21 +378,21 @@ func getRetry(
 		return baseRetry
 	}
 
-	if scenDefaults != nil && scenDefaults.Retry != nil {
-		scenRetry := scenDefaults.Retry
-		if scenRetry == api.NoRetry {
-			return scenRetry
+	if defaults != nil && defaults.Retry != nil {
+		defaultRetry := defaults.Retry
+		if defaultRetry == api.NoRetry {
+			return defaultRetry
 		}
 		msg := "using retry"
-		if scenRetry.Attempts != nil {
-			msg += fmt.Sprintf(" (attempts: %d)", *scenRetry.Attempts)
+		if defaultRetry.Attempts != nil {
+			msg += fmt.Sprintf(" (attempts: %d)", *defaultRetry.Attempts)
 		}
-		if scenRetry.Interval != "" {
-			msg += fmt.Sprintf(" (interval: %s)", scenRetry.Interval)
+		if defaultRetry.Interval != "" {
+			msg += fmt.Sprintf(" (interval: %s)", defaultRetry.Interval)
 		}
-		msg += fmt.Sprintf(" (exponential: %t) [scenario default]", scenRetry.Exponential)
+		msg += fmt.Sprintf(" (exponential: %t) [scenario default]", defaultRetry.Exponential)
 		debug.Println(ctx, msg)
-		return scenRetry
+		return defaultRetry
 	}
 
 	pluginInfo := plugin.Info()
@@ -401,9 +416,9 @@ func getRetry(
 	return nil
 }
 
-// getScenarioDefaults returns the Defaults parsed from the scenario's YAML
+// getDefaults returns the Defaults parsed from the scenario's YAML
 // file's `defaults` field, or nil if none were specified.
-func (s *Scenario) getScenarioDefaults() *Defaults {
+func (s *Scenario) getDefaults() *Defaults {
 	scDefaultsAny, found := s.Defaults[DefaultsKey]
 	if found {
 		return scDefaultsAny.(*Defaults)
diff --git a/scenario/run_test.go b/scenario/run_test.go
@@ -193,7 +193,7 @@ func TestNoRetry(t *testing.T) {
 	w.Flush()
 	require.NotEqual(b.Len(), 0)
 	debugout := b.String()
-	require.Contains(debugout, "[gdt] [no-retry/0:bar] run: single-shot (no retries) ok: true")
+	require.Contains(debugout, "[gdt] [no-retry/0:bar] spec/run: single-shot (no retries) ok: true")
 }
 
 func TestNoRetryEvaluableOverride(t *testing.T) {
@@ -217,7 +217,7 @@ func TestNoRetryEvaluableOverride(t *testing.T) {
 	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")
+	require.Contains(debugout, "[gdt] [no-retry-evaluable-override/0:bar] spec/run: single-shot (no retries) ok: true")
 }
 
 func TestFailRetryTestOverride(t *testing.T) {
@@ -253,7 +253,7 @@ func TestRetryTestOverride(t *testing.T) {
 	require.NotNil(err)
 
 	debugout := string(outerr)
-	require.Contains(debugout, "[gdt] [retry-test-override/0:baz] run: exceeded max attempts 2. stopping.")
+	require.Contains(debugout, "[gdt] [retry-test-override/0:baz] spec/run: exceeded max attempts 2. stopping.")
 }
 
 func TestSkipIf(t *testing.T) {
