From fdd76b91098aa6c9f113d2611e1ba18a367f3739 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Sep 2023 14:15:04 +0100 Subject: [PATCH 1/3] fix(scaletest/workspacetraffic): TestRun: wait for non-zero metrics before cancelling --- scaletest/workspacetraffic/run_test.go | 79 +++++++++++++++++++------- 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/scaletest/workspacetraffic/run_test.go b/scaletest/workspacetraffic/run_test.go index 308630910427d..5be28eef112d3 100644 --- a/scaletest/workspacetraffic/run_test.go +++ b/scaletest/workspacetraffic/run_test.go @@ -97,7 +97,6 @@ func TestRun(t *testing.T) { var ( bytesPerTick = 1024 tickInterval = 1000 * time.Millisecond - cancelAfter = 1500 * time.Millisecond fudgeWrite = 12 // The ReconnectingPTY payload incurs some overhead readMetrics = &testMetrics{} writeMetrics = &testMetrics{} @@ -113,12 +112,32 @@ func TestRun(t *testing.T) { }) var logs strings.Builder - // Stop the test after one 'tick'. This will cause an EOF. + + runDone := make(chan struct{}) go func() { - <-time.After(cancelAfter) - cancel() + defer close(runDone) + err := runner.Run(ctx, "", &logs) + assert.NoError(t, err, "unexpected error calling Run()") + }() + + gotMetrics := make(chan struct{}) + go func() { + defer close(gotMetrics) + // Wait until we get some non-zero metrics before canceling. + assert.Eventually(t, func() bool { + readLatencies := readMetrics.Latencies() + writeLatencies := writeMetrics.Latencies() + return len(readLatencies) > 0 && + len(writeLatencies) > 0 && + anyF(readLatencies, func(f float64) bool { return f > 0.0 }) && + anyF(writeLatencies, func(f float64) bool { return f > 0.0 }) + }, testutil.WaitLong, testutil.IntervalMedium, "expected non-zero metrics") }() - require.NoError(t, runner.Run(ctx, "", &logs), "unexpected error calling Run()") + + // Stop the test after we get some non-zero metrics. + <-gotMetrics + cancel() + <-runDone t.Logf("read errors: %.0f\n", readMetrics.Errors()) t.Logf("write errors: %.0f\n", writeMetrics.Errors()) @@ -132,12 +151,6 @@ func TestRun(t *testing.T) { assert.NotZero(t, readMetrics.Total()) // Latency should report non-zero values. assert.NotEmpty(t, readMetrics.Latencies()) - for _, l := range readMetrics.Latencies()[1:] { // skip the first one, which is always zero - assert.NotZero(t, l) - } - for _, l := range writeMetrics.Latencies()[1:] { // skip the first one, which is always zero - assert.NotZero(t, l) - } assert.NotEmpty(t, writeMetrics.Latencies()) // Should not report any errors! assert.Zero(t, readMetrics.Errors()) @@ -210,7 +223,6 @@ func TestRun(t *testing.T) { var ( bytesPerTick = 1024 tickInterval = 1000 * time.Millisecond - cancelAfter = 1500 * time.Millisecond fudgeWrite = 2 // We send \r\n, which is two bytes readMetrics = &testMetrics{} writeMetrics = &testMetrics{} @@ -226,12 +238,32 @@ func TestRun(t *testing.T) { }) var logs strings.Builder - // Stop the test after one 'tick'. This will cause an EOF. + + runDone := make(chan struct{}) go func() { - <-time.After(cancelAfter) - cancel() + defer close(runDone) + err := runner.Run(ctx, "", &logs) + assert.NoError(t, err, "unexpected error calling Run()") }() - require.NoError(t, runner.Run(ctx, "", &logs), "unexpected error calling Run()") + + gotMetrics := make(chan struct{}) + go func() { + defer close(gotMetrics) + // Wait until we get some non-zero metrics before canceling. + assert.Eventually(t, func() bool { + readLatencies := readMetrics.Latencies() + writeLatencies := writeMetrics.Latencies() + return len(readLatencies) > 0 && + len(writeLatencies) > 0 && + anyF(readLatencies, func(f float64) bool { return f > 0.0 }) && + anyF(writeLatencies, func(f float64) bool { return f > 0.0 }) + }, testutil.WaitLong, testutil.IntervalMedium, "expected non-zero metrics") + }() + + // Stop the test after we get some non-zero metrics. + <-gotMetrics + cancel() + <-runDone t.Logf("read errors: %.0f\n", readMetrics.Errors()) t.Logf("write errors: %.0f\n", writeMetrics.Errors()) @@ -245,12 +277,6 @@ func TestRun(t *testing.T) { assert.NotZero(t, readMetrics.Total()) // Latency should report non-zero values. assert.NotEmpty(t, readMetrics.Latencies()) - for _, l := range readMetrics.Latencies()[1:] { // skip the first one, which is always zero - assert.NotZero(t, l) - } - for _, l := range writeMetrics.Latencies()[1:] { // skip the first one, which is always zero - assert.NotZero(t, l) - } assert.NotEmpty(t, writeMetrics.Latencies()) // Should not report any errors! assert.Zero(t, readMetrics.Errors()) @@ -302,3 +328,12 @@ func (m *testMetrics) Latencies() []float64 { defer m.Unlock() return m.latencies } + +func anyF[T any](vals []T, fn func(T) bool) bool { + for _, val := range vals { + if fn(val) { + return true + } + } + return false +} From 95da6eb7157e7353e0251c02b441d0932476704e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Sep 2023 15:27:10 +0100 Subject: [PATCH 2/3] slices.ContainsFunc <3 --- scaletest/workspacetraffic/run_test.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/scaletest/workspacetraffic/run_test.go b/scaletest/workspacetraffic/run_test.go index 5be28eef112d3..cf6f9ce2efa81 100644 --- a/scaletest/workspacetraffic/run_test.go +++ b/scaletest/workspacetraffic/run_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/scaletest/workspacetraffic" "github.com/coder/coder/v2/testutil" + "golang.org/x/exp/slices" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -129,8 +130,8 @@ func TestRun(t *testing.T) { writeLatencies := writeMetrics.Latencies() return len(readLatencies) > 0 && len(writeLatencies) > 0 && - anyF(readLatencies, func(f float64) bool { return f > 0.0 }) && - anyF(writeLatencies, func(f float64) bool { return f > 0.0 }) + slices.ContainsFunc(readLatencies, func(f float64) bool { return f > 0.0 }) && + slices.ContainsFunc(writeLatencies, func(f float64) bool { return f > 0.0 }) }, testutil.WaitLong, testutil.IntervalMedium, "expected non-zero metrics") }() @@ -328,12 +329,3 @@ func (m *testMetrics) Latencies() []float64 { defer m.Unlock() return m.latencies } - -func anyF[T any](vals []T, fn func(T) bool) bool { - for _, val := range vals { - if fn(val) { - return true - } - } - return false -} From 6c0abee52c30c1df5248faa560add5c2c70b833e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Sep 2023 15:35:42 +0100 Subject: [PATCH 3/3] fixup! slices.ContainsFunc <3 --- scaletest/workspacetraffic/run_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scaletest/workspacetraffic/run_test.go b/scaletest/workspacetraffic/run_test.go index cf6f9ce2efa81..6e759cf46ebaf 100644 --- a/scaletest/workspacetraffic/run_test.go +++ b/scaletest/workspacetraffic/run_test.go @@ -256,8 +256,8 @@ func TestRun(t *testing.T) { writeLatencies := writeMetrics.Latencies() return len(readLatencies) > 0 && len(writeLatencies) > 0 && - anyF(readLatencies, func(f float64) bool { return f > 0.0 }) && - anyF(writeLatencies, func(f float64) bool { return f > 0.0 }) + slices.ContainsFunc(readLatencies, func(f float64) bool { return f > 0.0 }) && + slices.ContainsFunc(writeLatencies, func(f float64) bool { return f > 0.0 }) }, testutil.WaitLong, testutil.IntervalMedium, "expected non-zero metrics") }()