From 175590225c59f1bbb86f3113302c9fa020b5e470 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Sun, 14 May 2023 16:48:12 +0000 Subject: [PATCH] chore: reduce the log output of skipped tests With the introduction of the workspace proxy tests there was a lot of output if a test was eventually skipped. --- agent/agent_test.go | 2 +- coderd/database/migrations/migrate_test.go | 4 ++-- coderd/database/postgres/postgres_test.go | 2 +- coderd/database/pubsub_test.go | 2 +- coderd/workspaceapps/apptest/apptest.go | 28 ++++++++++++---------- coderd/workspaceapps/apptest/setup.go | 5 ---- coderd/workspaceapps_test.go | 11 ++++----- enterprise/wsproxy/wsproxy_test.go | 11 ++++----- 8 files changed, 31 insertions(+), 34 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 8914a5524f4ec..1dab56c2c7696 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1020,7 +1020,7 @@ func TestAgentMetadata_Timing(t *testing.T) { if runtime.GOOS == "windows" { // Shell scripting in Windows is a pain, and we have already tested // that the OS logic works in the simpler tests. - t.Skip() + t.SkipNow() } testutil.SkipIfNotTiming(t) t.Parallel() diff --git a/coderd/database/migrations/migrate_test.go b/coderd/database/migrations/migrate_test.go index ae0b2791fa42a..263d51871e3c4 100644 --- a/coderd/database/migrations/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -34,7 +34,7 @@ func TestMigrate(t *testing.T) { t.Parallel() if testing.Short() { - t.Skip() + t.SkipNow() return } @@ -198,7 +198,7 @@ func TestMigrateUpWithFixtures(t *testing.T) { t.Parallel() if testing.Short() { - t.Skip() + t.SkipNow() return } diff --git a/coderd/database/postgres/postgres_test.go b/coderd/database/postgres/postgres_test.go index d11fdb21e89c0..88d9800a57644 100644 --- a/coderd/database/postgres/postgres_test.go +++ b/coderd/database/postgres/postgres_test.go @@ -23,7 +23,7 @@ func TestPostgres(t *testing.T) { // t.Parallel() if testing.Short() { - t.Skip() + t.SkipNow() return } diff --git a/coderd/database/pubsub_test.go b/coderd/database/pubsub_test.go index 13d3c5723fb29..0abd0c8ca1177 100644 --- a/coderd/database/pubsub_test.go +++ b/coderd/database/pubsub_test.go @@ -24,7 +24,7 @@ func TestPubsub(t *testing.T) { t.Parallel() if testing.Short() { - t.Skip() + t.SkipNow() return } diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go index ab90b0a4b43bf..664e8ce073ed1 100644 --- a/coderd/workspaceapps/apptest/apptest.go +++ b/coderd/workspaceapps/apptest/apptest.go @@ -32,7 +32,12 @@ import ( // Run runs the entire workspace app test suite against deployments minted // by the provided factory. -func Run(t *testing.T, factory DeploymentFactory) { +// +// appHostIsPrimary is true if the app host is also the primary coder API +// server. This disables any tests that test API passthrough or rely on the +// app server not being the API server. +// nolint:revive +func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) { setupProxyTest := func(t *testing.T, opts *DeploymentOptions) *Details { return setupProxyTestWithFactory(t, factory, opts) } @@ -109,12 +114,12 @@ func Run(t *testing.T, factory DeploymentFactory) { t.Run("SignedTokenQueryParameter", func(t *testing.T) { t.Parallel() - - appDetails := setupProxyTest(t, nil) - if appDetails.AppHostIsPrimary { + if appHostIsPrimary { t.Skip("Tickets are not used for terminal requests on the primary.") } + appDetails := setupProxyTest(t, nil) + u := *appDetails.PathAppBaseURL if u.Scheme == "http" { u.Scheme = "ws" @@ -197,7 +202,7 @@ func Run(t *testing.T, factory DeploymentFactory) { t.Run("LoginWithoutAuthOnPrimary", func(t *testing.T) { t.Parallel() - if !appDetails.AppHostIsPrimary { + if !appHostIsPrimary { t.Skip("This test only applies when testing apps on the primary.") } @@ -222,7 +227,7 @@ func Run(t *testing.T, factory DeploymentFactory) { t.Run("LoginWithoutAuthOnProxy", func(t *testing.T) { t.Parallel() - if appDetails.AppHostIsPrimary { + if appHostIsPrimary { t.Skip("This test only applies when testing apps on workspace proxies.") } @@ -448,7 +453,7 @@ func Run(t *testing.T, factory DeploymentFactory) { for _, c := range cases { c := c - if c.name == "Path" && appDetails.AppHostIsPrimary { + if c.name == "Path" && appHostIsPrimary { // Workspace application auth does not apply to path apps // served from the primary access URL as no smuggling needs // to take place (they're already logged in with a session @@ -599,16 +604,15 @@ func Run(t *testing.T, factory DeploymentFactory) { // --app-hostname is not set by the admin. t.Run("WorkspaceAppsProxySubdomainPassthrough", func(t *testing.T) { t.Parallel() - + if !appHostIsPrimary { + t.Skip("app hostname does not serve API") + } // No Hostname set. appDetails := setupProxyTest(t, &DeploymentOptions{ AppHost: "", DisableSubdomainApps: true, noWorkspace: true, }) - if !appDetails.AppHostIsPrimary { - t.Skip("app hostname does not serve API") - } ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() @@ -1031,7 +1035,7 @@ func Run(t *testing.T, factory DeploymentFactory) { require.NoError(t, err, msg) expectedPath := "/login" - if !isPathApp || !appDetails.AppHostIsPrimary { + if !isPathApp || !appHostIsPrimary { expectedPath = "/api/v2/applications/auth-redirect" } assert.Equal(t, expectedPath, location.Path, "should not have access, expected redirect to applicable login endpoint. "+msg) diff --git a/coderd/workspaceapps/apptest/setup.go b/coderd/workspaceapps/apptest/setup.go index 29815dc55c5ae..60a1ad76cabbb 100644 --- a/coderd/workspaceapps/apptest/setup.go +++ b/coderd/workspaceapps/apptest/setup.go @@ -63,11 +63,6 @@ type Deployment struct { SDKClient *codersdk.Client FirstUser codersdk.CreateFirstUserResponse PathAppBaseURL *url.URL - - // AppHostIsPrimary is true if the app host is also the primary coder API - // server. This disables any tests that test API passthrough or rely on the - // app server not being the API server. - AppHostIsPrimary bool } // DeploymentFactory generates a deployment with an API client, a path base URL, diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 26db0f393efa5..8f31bed4d27d2 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -252,7 +252,7 @@ func TestWorkspaceApplicationAuth(t *testing.T) { func TestWorkspaceApps(t *testing.T) { t.Parallel() - apptest.Run(t, func(t *testing.T, opts *apptest.DeploymentOptions) *apptest.Deployment { + apptest.Run(t, true, func(t *testing.T, opts *apptest.DeploymentOptions) *apptest.Deployment { deploymentValues := coderdtest.DeploymentValues(t) deploymentValues.DisablePathApps = clibase.Bool(opts.DisablePathApps) deploymentValues.Dangerous.AllowPathAppSharing = clibase.Bool(opts.DangerousAllowPathAppSharing) @@ -280,11 +280,10 @@ func TestWorkspaceApps(t *testing.T) { user := coderdtest.CreateFirstUser(t, client) return &apptest.Deployment{ - Options: opts, - SDKClient: client, - FirstUser: user, - PathAppBaseURL: client.URL, - AppHostIsPrimary: true, + Options: opts, + SDKClient: client, + FirstUser: user, + PathAppBaseURL: client.URL, } }) } diff --git a/enterprise/wsproxy/wsproxy_test.go b/enterprise/wsproxy/wsproxy_test.go index 6b4ef67bbfeb1..2c5ce4405b719 100644 --- a/enterprise/wsproxy/wsproxy_test.go +++ b/enterprise/wsproxy/wsproxy_test.go @@ -16,7 +16,7 @@ import ( func TestWorkspaceProxyWorkspaceApps(t *testing.T) { t.Parallel() - apptest.Run(t, func(t *testing.T, opts *apptest.DeploymentOptions) *apptest.Deployment { + apptest.Run(t, false, func(t *testing.T, opts *apptest.DeploymentOptions) *apptest.Deployment { deploymentValues := coderdtest.DeploymentValues(t) deploymentValues.DisablePathApps = clibase.Bool(opts.DisablePathApps) deploymentValues.Dangerous.AllowPathAppSharing = clibase.Bool(opts.DangerousAllowPathAppSharing) @@ -61,11 +61,10 @@ func TestWorkspaceProxyWorkspaceApps(t *testing.T) { }) return &apptest.Deployment{ - Options: opts, - SDKClient: client, - FirstUser: user, - PathAppBaseURL: proxyAPI.Options.AccessURL, - AppHostIsPrimary: false, + Options: opts, + SDKClient: client, + FirstUser: user, + PathAppBaseURL: proxyAPI.Options.AccessURL, } }) }