coder · kylecarbs · May 15, 2023 · May 14, 2023
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
 		}
 	})
 }