Skip to content

Commit

Permalink
break run spec and exec spec into separate methods
Browse files Browse the repository at this point in the history 
Cleans up the (large) Scenario.Run and runSpec methods by separating the
"preparation" phase of the test spec execution from the execution itself
into a different method. This allows us to clean up the manual `defer`s
and make the code more readable.

Signed-off-by: Jay Pipes <[email protected]>
  • Loading branch information
@jaypipes
jaypipes committed Jul 8, 2024
1 parent 0732fa0 commit a916cff
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 114 deletions.
7 changes: 4 additions & 3 deletions api/error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
231 changes: 123 additions & 108 deletions scenario/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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)
}
}
})
Expand All @@ -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,
Expand All @@ -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}
Expand Down Expand Up @@ -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()
Expand All @@ -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 {
Expand All @@ -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,
)
}
Expand All @@ -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:
//
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions scenario/run_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit a916cff

Please sign in to comment.