Sep 13, 2023 · Sep 13, 2023 · Sep 13, 2023 · Sep 13, 2023 · Sep 13, 2023 · Sep 13, 2023
diff --git a/scaletest/workspacetraffic/run_test.go b/scaletest/workspacetraffic/run_test.go
 "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"
 var (
 bytesPerTick = 1024
 tickInterval = 1000 * time.Millisecond
 cancelAfter  = 1500 * time.Millisecond
 fudgeWrite   = 12 // The ReconnectingPTY payload incurs some overhead
 readMetrics  = &testMetrics{}
 writeMetrics = &testMetrics{}
 })

 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 &&
 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")
 }()
 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())
 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())
 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{}
 })

 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 &&
 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")
 }()
 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())
 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())
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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"
Expand DownExpand Up		@@ -97,7 +98,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{}
Expand All		@@ -113,12 +113,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 &&
		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")
		}()
		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())
Expand All		@@ -132,12 +152,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())
Expand DownExpand Up		@@ -210,7 +224,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{}
Expand All		@@ -226,12 +239,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() {
Copy link Member mtojekSep 13, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. nit: I'm always confused if there is a possibility of a routine leak. defer goes first then routine is killed, right? Copy link MemberAuthor johnstcnSep 13, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Defer runs when the surrounding function returns. So`assert.Eventually()` has to finish before`gotMetrics` is closed, and`<-gotMetrics` will block until that happens. Then we cancel the context passed to`runner.Run`, which will cause it to exit, then we close`runDone`, which unblocked`<-runDone`. I think`goleak` would have caught a goroutine leak here as well. mtojek reacted with thumbs up emoji
		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 &&
		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")
		}()
		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())
Expand All		@@ -245,12 +278,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())
Expand Down