diff --git a/cli/server_test.go b/cli/server_test.go index 6cd13cb32e..1683ea6647 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -35,7 +35,6 @@ import ( "github.com/coder/coder/coderd/database/postgres" "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/codersdk" - "github.com/coder/coder/cryptorand" "github.com/coder/coder/pty/ptytest" "github.com/coder/coder/testutil" ) @@ -1132,12 +1131,7 @@ func TestServer(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - random, err := cryptorand.String(5) - require.NoError(t, err) - fiName := fmt.Sprint(os.TempDir(), "/coder-logging-test-", random) - defer func() { - _ = os.Remove(fiName) - }() + fiName := testutil.TempFile(t, "", "coder-logging-test-*") root, _ := clitest.New(t, "server", @@ -1165,11 +1159,7 @@ func TestServer(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - fi, err := os.CreateTemp("", "coder-logging-test-*") - require.NoError(t, err) - defer func() { - _ = os.Remove(fi.Name()) - }() + fi := testutil.TempFile(t, "", "coder-logging-test-*") root, _ := clitest.New(t, "server", @@ -1177,7 +1167,7 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--log-human", fi.Name(), + "--log-human", fi, ) serverErr := make(chan error, 1) go func() { @@ -1185,7 +1175,7 @@ func TestServer(t *testing.T) { }() assert.Eventually(t, func() bool { - stat, err := os.Stat(fi.Name()) + stat, err := os.Stat(fi) return err == nil && stat.Size() > 0 }, testutil.WaitShort, testutil.IntervalFast) cancelFunc() @@ -1197,11 +1187,7 @@ func TestServer(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - fi, err := os.CreateTemp("", "coder-logging-test-*") - require.NoError(t, err) - defer func() { - _ = os.Remove(fi.Name()) - }() + fi := testutil.TempFile(t, "", "coder-logging-test-*") root, _ := clitest.New(t, "server", @@ -1209,7 +1195,7 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--log-json", fi.Name(), + "--log-json", fi, ) serverErr := make(chan error, 1) go func() { @@ -1217,7 +1203,7 @@ func TestServer(t *testing.T) { }() assert.Eventually(t, func() bool { - stat, err := os.Stat(fi.Name()) + stat, err := os.Stat(fi) return err == nil && stat.Size() > 0 }, testutil.WaitShort, testutil.IntervalFast) cancelFunc() @@ -1229,11 +1215,7 @@ func TestServer(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - fi, err := os.CreateTemp("", "coder-logging-test-*") - require.NoError(t, err) - defer func() { - _ = os.Remove(fi.Name()) - }() + fi := testutil.TempFile(t, "", "coder-logging-test-*") root, _ := clitest.New(t, "server", @@ -1241,19 +1223,32 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--log-stackdriver", fi.Name(), + "--log-stackdriver", fi, ) + // Attach pty so we get debug output from the command if this test + // fails. + pty := ptytest.New(t) + root.SetOut(pty.Output()) + root.SetErr(pty.Output()) + serverErr := make(chan error, 1) go func() { serverErr <- root.ExecuteContext(ctx) }() + defer func() { + cancelFunc() + <-serverErr + }() - assert.Eventually(t, func() bool { - stat, err := os.Stat(fi.Name()) + require.Eventually(t, func() bool { + line := pty.ReadLine() + return strings.HasPrefix(line, "Started HTTP listener at ") + }, testutil.WaitLong*2, testutil.IntervalMedium, "wait for server to listen on http") + + require.Eventually(t, func() bool { + stat, err := os.Stat(fi) return err == nil && stat.Size() > 0 }, testutil.WaitLong, testutil.IntervalMedium) - cancelFunc() - <-serverErr }) t.Run("Multiple", func(t *testing.T) { @@ -1261,54 +1256,56 @@ func TestServer(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - fi1, err := os.CreateTemp("", "coder-logging-test-*") - require.NoError(t, err) - defer func() { - _ = os.Remove(fi1.Name()) - }() - - fi2, err := os.CreateTemp("", "coder-logging-test-*") - require.NoError(t, err) - defer func() { - _ = os.Remove(fi2.Name()) - }() - - fi3, err := os.CreateTemp("", "coder-logging-test-*") - require.NoError(t, err) - defer func() { - _ = os.Remove(fi3.Name()) - }() + fi1 := testutil.TempFile(t, "", "coder-logging-test-*") + fi2 := testutil.TempFile(t, "", "coder-logging-test-*") + fi3 := testutil.TempFile(t, "", "coder-logging-test-*") + // NOTE(mafredri): This test might end up downloading Terraform + // which can take a long time and end up failing the test. + // This is why we wait extra long below for server to listen on + // HTTP. root, _ := clitest.New(t, "server", "--verbose", "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--log-human", fi1.Name(), - "--log-json", fi2.Name(), - "--log-stackdriver", fi3.Name(), + "--log-human", fi1, + "--log-json", fi2, + "--log-stackdriver", fi3, ) + // Attach pty so we get debug output from the command if this test + // fails. + pty := ptytest.New(t) + root.SetOut(pty.Output()) + root.SetErr(pty.Output()) + serverErr := make(chan error, 1) go func() { serverErr <- root.ExecuteContext(ctx) }() + defer func() { + cancelFunc() + <-serverErr + }() - assert.Eventually(t, func() bool { - stat, err := os.Stat(fi1.Name()) - return err == nil && stat.Size() > 0 - }, testutil.WaitLong, testutil.IntervalMedium) - assert.Eventually(t, func() bool { - stat, err := os.Stat(fi2.Name()) - return err == nil && stat.Size() > 0 - }, testutil.WaitLong, testutil.IntervalMedium) - assert.Eventually(t, func() bool { - stat, err := os.Stat(fi3.Name()) - return err == nil && stat.Size() > 0 - }, testutil.WaitLong, testutil.IntervalMedium) + require.Eventually(t, func() bool { + line := pty.ReadLine() + return strings.HasPrefix(line, "Started HTTP listener at ") + }, testutil.WaitLong*2, testutil.IntervalMedium, "wait for server to listen on http") - cancelFunc() - <-serverErr + require.Eventually(t, func() bool { + stat, err := os.Stat(fi1) + return err == nil && stat.Size() > 0 + }, testutil.WaitShort, testutil.IntervalMedium, "log human size > 0") + require.Eventually(t, func() bool { + stat, err := os.Stat(fi2) + return err == nil && stat.Size() > 0 + }, testutil.WaitShort, testutil.IntervalMedium, "log json size > 0") + require.Eventually(t, func() bool { + stat, err := os.Stat(fi3) + return err == nil && stat.Size() > 0 + }, testutil.WaitShort, testutil.IntervalMedium, "log stackdriver size > 0") }) }) } diff --git a/testutil/temp.go b/testutil/temp.go new file mode 100644 index 0000000000..4f17552415 --- /dev/null +++ b/testutil/temp.go @@ -0,0 +1,47 @@ +package testutil + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +// TempFile returns the name of a temporary file that does not exist. +func TempFile(t *testing.T, dir, pattern string) string { + t.Helper() + + if dir == "" { + dir = t.TempDir() + } + f, err := os.CreateTemp(dir, pattern) + require.NoError(t, err, "create temp file") + name := f.Name() + err = f.Close() + require.NoError(t, err, "close temp file") + err = os.Remove(name) + require.NoError(t, err, "remove temp file") + + return name +} + +// CreateTemp is a convenience function for creating a temporary file, like +// os.CreateTemp, but it also registers a cleanup function to close and remove +// the file. +func CreateTemp(t *testing.T, dir, pattern string) *os.File { + t.Helper() + + if dir == "" { + dir = t.TempDir() + } + f, err := os.CreateTemp(dir, pattern) + require.NoError(t, err, "create temp file") + t.Cleanup(func() { + _ = f.Close() + err = os.Remove(f.Name()) + if err != nil { + t.Logf("CreateTemp: Cleanup: remove failed for %q: %v", f.Name(), err) + } + }) + return f +}