Skip to content

Commit

Permalink
feat: add cleanup strategy to loadtest (#4991)
Browse files Browse the repository at this point in the history
  • Loading branch information
@deansheather
deansheather committed Nov 10, 2022
1 parent 1c9677d commit e847276
Show file tree
Hide file tree
Showing 9 changed files with 314 additions and 122 deletions.
5 changes: 3 additions & 2 deletions cli/loadtest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ func loadtest() *cobra.Command {
client.PropagateTracing = tracePropagate

// Prepare the test.
strategy := config.Strategy.ExecutionStrategy()
th := harness.NewTestHarness(strategy)
runStrategy := config.Strategy.ExecutionStrategy()
cleanupStrategy := config.CleanupStrategy.ExecutionStrategy()
th := harness.NewTestHarness(runStrategy, cleanupStrategy)

for i, t := range config.Tests {
name := fmt.Sprintf("%s-%d", t.Type, i)
Expand Down
10 changes: 10 additions & 0 deletions cli/loadtest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ func TestLoadTest(t *testing.T) {
Strategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
CleanupStrategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
Tests: []cli.LoadTest{
{
Type: cli.LoadTestTypePlacebo,
Expand Down Expand Up @@ -89,6 +92,10 @@ func TestLoadTest(t *testing.T) {
Type: cli.LoadTestStrategyTypeConcurrent,
ConcurrencyLimit: 2,
},
CleanupStrategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeConcurrent,
ConcurrencyLimit: 2,
},
Tests: []cli.LoadTest{
{
Type: cli.LoadTestTypeWorkspaceBuild,
Expand Down Expand Up @@ -210,6 +217,9 @@ func TestLoadTest(t *testing.T) {
Strategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
CleanupStrategy: cli.LoadTestStrategy{
Type: cli.LoadTestStrategyTypeLinear,
},
Tests: []cli.LoadTest{
{
Type: cli.LoadTestTypePlacebo,
Expand Down
9 changes: 7 additions & 2 deletions cli/loadtestconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import (

// LoadTestConfig is the overall configuration for a call to `coder loadtest`.
type LoadTestConfig struct {
Strategy LoadTestStrategy `json:"strategy"`
Tests []LoadTest `json:"tests"`
Strategy LoadTestStrategy `json:"strategy"`
CleanupStrategy LoadTestStrategy `json:"cleanup_strategy"`
Tests []LoadTest `json:"tests"`
// Timeout sets a timeout for the entire test run, to control the timeout
// for each individual run use strategy.timeout.
Timeout httpapi.Duration `json:"timeout"`
Expand Down Expand Up @@ -134,6 +135,10 @@ func (c *LoadTestConfig) Validate() error {
if err != nil {
return xerrors.Errorf("validate strategy: %w", err)
}
err = c.CleanupStrategy.Validate()
if err != nil {
return xerrors.Errorf("validate cleanup_strategy: %w", err)
}

for i, test := range c.Tests {
err := test.Validate()
Expand Down
66 changes: 40 additions & 26 deletions loadtest/harness/harness.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,11 @@ import (
"github.com/coder/coder/coderd/tracing"
)

// ExecutionStrategy defines how a TestHarness should execute a set of runs. It
// essentially defines the concurrency model for a given testing session.
type ExecutionStrategy interface {
// Execute runs the given runs in whatever way the strategy wants. An error
// may only be returned if the strategy has a failure itself, not if any of
// the runs fail.
Execute(ctx context.Context, runs []*TestRun) error
}

// TestHarness runs a bunch of registered test runs using the given
// ExecutionStrategy.
// TestHarness runs a bunch of registered test runs using the given execution
// strategies.
type TestHarness struct {
strategy ExecutionStrategy
runStrategy ExecutionStrategy
cleanupStrategy ExecutionStrategy

mut *sync.Mutex
runIDs map[string]struct{}
Expand All @@ -33,14 +25,15 @@ type TestHarness struct {
elapsed time.Duration
}

// NewTestHarness creates a new TestHarness with the given ExecutionStrategy.
func NewTestHarness(strategy ExecutionStrategy) *TestHarness {
// NewTestHarness creates a new TestHarness with the given execution strategies.
func NewTestHarness(runStrategy, cleanupStrategy ExecutionStrategy) *TestHarness {
return &TestHarness{
strategy: strategy,
mut: new(sync.Mutex),
runIDs: map[string]struct{}{},
runs: []*TestRun{},
done: make(chan struct{}),
runStrategy: runStrategy,
cleanupStrategy: cleanupStrategy,
mut: new(sync.Mutex),
runIDs: map[string]struct{}{},
runs: []*TestRun{},
done: make(chan struct{}),
}
}

Expand All @@ -62,11 +55,16 @@ func (h *TestHarness) Run(ctx context.Context) (err error) {
h.started = true
h.mut.Unlock()

runFns := make([]TestFn, len(h.runs))
for i, run := range h.runs {
runFns[i] = run.Run
}

defer close(h.done)
defer func() {
e := recover()
if e != nil {
err = xerrors.Errorf("execution strategy panicked: %w", e)
err = xerrors.Errorf("panic in harness.Run: %+v", e)
}
}()

Expand All @@ -77,7 +75,9 @@ func (h *TestHarness) Run(ctx context.Context) (err error) {
h.elapsed = time.Since(start)
}()

err = h.strategy.Execute(ctx, h.runs)
// We don't care about test failures here since they already get recorded
// by the *TestRun.
_, err = h.runStrategy.Run(ctx, runFns)
//nolint:revive // we use named returns because we mutate it in a defer
return
}
Expand All @@ -96,20 +96,34 @@ func (h *TestHarness) Cleanup(ctx context.Context) (err error) {
panic("harness has not finished")
}

cleanupFns := make([]TestFn, len(h.runs))
for i, run := range h.runs {
cleanupFns[i] = run.Cleanup
}

defer func() {
e := recover()
if e != nil {
err = multierror.Append(err, xerrors.Errorf("panic in cleanup: %w", e))
err = xerrors.Errorf("panic in harness.Cleanup: %+v", e)
}
}()

for _, run := range h.runs {
e := run.Cleanup(ctx)
if e != nil {
err = multierror.Append(err, xerrors.Errorf("cleanup for %s failed: %w", run.FullID(), e))
var cleanupErrs []error
cleanupErrs, err = h.cleanupStrategy.Run(ctx, cleanupFns)
if err != nil {
err = xerrors.Errorf("cleanup strategy error: %w", err)
//nolint:revive // we use named returns because we mutate it in a defer
return
}

var merr error
for _, cleanupErr := range cleanupErrs {
if cleanupErr != nil {
merr = multierror.Append(merr, cleanupErr)
}
}

err = merr
//nolint:revive // we use named returns because we mutate it in a defer
return
}
70 changes: 54 additions & 16 deletions loadtest/harness/harness_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type panickingExecutionStrategy struct{}

var _ harness.ExecutionStrategy = panickingExecutionStrategy{}

func (panickingExecutionStrategy) Execute(_ context.Context, _ []*harness.TestRun) error {
func (panickingExecutionStrategy) Run(_ context.Context, _ []harness.TestFn) ([]error, error) {
panic(testPanicMessage)
}

Expand All @@ -28,8 +28,8 @@ type erroringExecutionStrategy struct {

var _ harness.ExecutionStrategy = erroringExecutionStrategy{}

func (e erroringExecutionStrategy) Execute(_ context.Context, _ []*harness.TestRun) error {
return e.err
func (e erroringExecutionStrategy) Run(_ context.Context, _ []harness.TestFn) ([]error, error) {
return []error{}, e.err
}

func Test_TestHarness(t *testing.T) {
Expand All @@ -40,7 +40,7 @@ func Test_TestHarness(t *testing.T) {

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
r1 := h.AddRun("test", "1", fakeTestFns(nil, nil))
r2 := h.AddRun("test", "2", fakeTestFns(expectedErr, nil))

Expand All @@ -65,7 +65,7 @@ func Test_TestHarness(t *testing.T) {

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(erroringExecutionStrategy{err: expectedErr})
h := harness.NewTestHarness(erroringExecutionStrategy{err: expectedErr}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", fakeTestFns(nil, nil))

err := h.Run(context.Background())
Expand All @@ -76,7 +76,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("CatchesExecutionPanic", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(panickingExecutionStrategy{})
h := harness.NewTestHarness(panickingExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", fakeTestFns(nil, nil))

err := h.Run(context.Background())
Expand All @@ -93,7 +93,7 @@ func Test_TestHarness(t *testing.T) {

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", fakeTestFns(nil, expectedErr))

err := h.Run(context.Background())
Expand All @@ -107,7 +107,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("Panic", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
return nil
Expand All @@ -125,6 +125,44 @@ func Test_TestHarness(t *testing.T) {
require.ErrorContains(t, err, "panic")
require.ErrorContains(t, err, testPanicMessage)
})

t.Run("CatchesExecutionError", func(t *testing.T) {
t.Parallel()

expectedErr := xerrors.New("expected error")

h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, erroringExecutionStrategy{err: expectedErr})
_ = h.AddRun("test", "1", fakeTestFns(nil, nil))

err := h.Run(context.Background())
require.NoError(t, err)

err = h.Cleanup(context.Background())
require.Error(t, err)
require.ErrorIs(t, err, expectedErr)
})

t.Run("CatchesExecutionPanic", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, panickingExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
return nil
},
CleanupFn: func(_ context.Context, _ string) error {
return nil
},
})

err := h.Run(context.Background())
require.NoError(t, err)

err = h.Cleanup(context.Background())
require.Error(t, err)
require.ErrorContains(t, err, "panic")
require.ErrorContains(t, err, testPanicMessage)
})
})

t.Run("Panics", func(t *testing.T) {
Expand All @@ -133,7 +171,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("RegisterAfterStart", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.Run(context.Background())

require.Panics(t, func() {
Expand All @@ -144,7 +182,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("DuplicateTestID", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})

name, id := "test", "1"
_ = h.AddRun(name, id, fakeTestFns(nil, nil))
Expand All @@ -157,7 +195,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("StartedTwice", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
h.Run(context.Background())

require.Panics(t, func() {
Expand All @@ -168,7 +206,7 @@ func Test_TestHarness(t *testing.T) {
t.Run("ResultsBeforeStart", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})

require.Panics(t, func() {
h.Results()
Expand All @@ -183,7 +221,7 @@ func Test_TestHarness(t *testing.T) {
endRun = make(chan struct{})
testsEnded = make(chan struct{})
)
h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
close(started)
Expand All @@ -210,22 +248,22 @@ func Test_TestHarness(t *testing.T) {
t.Run("CleanupBeforeStart", func(t *testing.T) {
t.Parallel()

h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})

require.Panics(t, func() {
h.Cleanup(context.Background())
})
})

t.Run("ClenaupBeforeFinish", func(t *testing.T) {
t.Run("CleanupBeforeFinish", func(t *testing.T) {
t.Parallel()

var (
started = make(chan struct{})
endRun = make(chan struct{})
testsEnded = make(chan struct{})
)
h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
h := harness.NewTestHarness(harness.LinearExecutionStrategy{}, harness.LinearExecutionStrategy{})
_ = h.AddRun("test", "1", testFns{
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
close(started)
Expand Down

0 comments on commit e847276

Please sign in to comment.