mirror of https://github.com/coder/coder.git
chore(coderd/workspaceapps/apptest): fix test flake due to concurrent usage of same deployment (#12700)
- Updates existing tests under workspaceapps/apptest to not reuse existing appDetails as assertWorkspaceLastUsed(Not)?Updated calls FlushStats() which was causing racy test behaviour and incorrect test assertions. - Expands scope of assertWorkspaceLastUsedAtUpdated and its counterpart to ProxySubdomain tests.
This commit is contained in:
Cian Johnston
GitHub
parent
5454f4997b
commit
8ea5fb7115
|
|
@ -70,6 +70,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
|
||||
t.Run("SignedTokenQueryParameter", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if appHostIsPrimary {
|
||||
t.Skip("Tickets are not used for terminal requests on the primary.")
|
||||
}
|
||||
|
|
@ -101,8 +102,6 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("WorkspaceAppsProxyPath", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
|
||||
t.Run("Disabled", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
|
@ -132,6 +131,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Skip("This test only applies when testing apps on the primary.")
|
||||
}
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
unauthedClient := appDetails.AppClient(t)
|
||||
unauthedClient.SetSessionToken("")
|
||||
|
||||
|
|
@ -148,7 +148,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
require.True(t, loc.Query().Has("message"))
|
||||
require.True(t, loc.Query().Has("redirect"))
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("LoginWithoutAuthOnProxy", func(t *testing.T) {
|
||||
|
|
@ -158,6 +158,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Skip("This test only applies when testing apps on workspace proxies.")
|
||||
}
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
unauthedClient := appDetails.AppClient(t)
|
||||
unauthedClient.SetSessionToken("")
|
||||
|
||||
|
|
@ -186,12 +187,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
// request is getting stripped.
|
||||
require.Equal(t, u.Path, redirectURI.Path+"/")
|
||||
require.Equal(t, u.RawQuery, redirectURI.RawQuery)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("NoAccessShould404", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
userClient, _ := coderdtest.CreateAnotherUser(t, appDetails.SDKClient, appDetails.FirstUser.OrganizationID, rbac.RoleMember())
|
||||
userAppClient := appDetails.AppClient(t)
|
||||
userAppClient.SetSessionToken(userClient.SessionToken())
|
||||
|
|
@ -210,6 +212,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("RedirectsWithSlash", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -226,6 +229,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("RedirectsWithQuery", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -245,6 +249,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("Proxies", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -333,6 +338,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("BlocksMe", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -347,13 +353,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
body, err := io.ReadAll(resp.Body)
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, string(body), "must be accessed with the full username, not @me")
|
||||
// TODO(cian): A blocked request should not count as workspace usage.
|
||||
// assertWorkspaceLastUsedAtNotUpdated(t, appDetails.AppClient(t), appDetails)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("ForwardsIP", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -373,6 +379,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("ProxyError", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -388,6 +395,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("NoProxyPort", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -397,7 +405,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
// TODO(@deansheather): This should be 400. There's a todo in the
|
||||
// resolve request code to fix this.
|
||||
require.Equal(t, http.StatusInternalServerError, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -636,6 +644,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
_ = resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
require.Equal(t, resp.Header.Get("X-Got-Host"), u.Host)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("WorkspaceAppsProxySubdomainHostnamePrefix/Different", func(t *testing.T) {
|
||||
|
|
@ -686,6 +695,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
_ = resp.Body.Close()
|
||||
require.NotEqual(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
// This test ensures that the subdomain handler does nothing if
|
||||
|
|
@ -754,11 +764,10 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
t.Run("WorkspaceAppsProxySubdomain", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
|
||||
t.Run("NoAccessShould401", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
userClient, _ := coderdtest.CreateAnotherUser(t, appDetails.SDKClient, appDetails.FirstUser.OrganizationID, rbac.RoleMember())
|
||||
userAppClient := appDetails.AppClient(t)
|
||||
userAppClient.SetSessionToken(userClient.SessionToken())
|
||||
|
|
@ -770,11 +779,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("RedirectsWithSlash", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -789,11 +800,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
loc, err := resp.Location()
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, appDetails.SubdomainAppURL(appDetails.Apps.Owner).Path, loc.Path)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("RedirectsWithQuery", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -807,11 +820,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
loc, err := resp.Location()
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, appDetails.SubdomainAppURL(appDetails.Apps.Owner).RawQuery, loc.RawQuery)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("Proxies", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -848,6 +863,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
require.Equal(t, proxyTestAppBody, string(body))
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("ProxiesHTTPS", func(t *testing.T) {
|
||||
|
|
@ -892,11 +908,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
require.Equal(t, proxyTestAppBody, string(body))
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("ProxiesPort", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -907,11 +925,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
require.Equal(t, proxyTestAppBody, string(body))
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("ProxyError", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -919,11 +939,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusBadGateway, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("ProxyPortMinimumError", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -939,6 +961,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
err = json.NewDecoder(resp.Body).Decode(&resBody)
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, resBody.Message, "Coder reserves ports less than")
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("SuffixWildcardOK", func(t *testing.T) {
|
||||
|
|
@ -961,6 +984,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
require.Equal(t, proxyTestAppBody, string(body))
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("WildcardPortOK", func(t *testing.T) {
|
||||
|
|
@ -993,16 +1017,19 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
require.Equal(t, proxyTestAppBody, string(body))
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("SuffixWildcardNotMatch", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, &DeploymentOptions{
|
||||
AppHost: "*-suffix.test.coder.com",
|
||||
})
|
||||
|
||||
t.Run("NoSuffix", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, &DeploymentOptions{
|
||||
AppHost: "*-suffix.test.coder.com",
|
||||
})
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -1020,11 +1047,16 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
// It's probably rendering the dashboard or a 404 page, so only
|
||||
// ensure that the body doesn't match.
|
||||
require.NotContains(t, string(body), proxyTestAppBody)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("DifferentSuffix", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, &DeploymentOptions{
|
||||
AppHost: "*-suffix.test.coder.com",
|
||||
})
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
|
|
@ -1042,6 +1074,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
// It's probably rendering the dashboard, so only ensure that the body
|
||||
// doesn't match.
|
||||
require.NotContains(t, string(body), proxyTestAppBody)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -1062,6 +1095,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("AuthenticatedOK", func(t *testing.T) {
|
||||
|
|
@ -1090,6 +1124,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("PublicOK", func(t *testing.T) {
|
||||
|
|
@ -1117,6 +1152,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
|
||||
t.Run("HTTPS", func(t *testing.T) {
|
||||
|
|
@ -1145,6 +1181,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
|||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assertWorkspaceLastUsedAtUpdated(t, appDetails)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -1719,26 +1756,33 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
|
|||
}
|
||||
|
||||
// Accessing an app should update the workspace's LastUsedAt.
|
||||
// NOTE: Despite our efforts with the flush channel, this is inherently racy.
|
||||
// NOTE: Despite our efforts with the flush channel, this is inherently racy when used with
|
||||
// parallel tests on the same workspace/app.
|
||||
func assertWorkspaceLastUsedAtUpdated(t testing.TB, details *Details) {
|
||||
t.Helper()
|
||||
|
||||
require.NotNil(t, details.Workspace, "can't assert LastUsedAt on a nil workspace!")
|
||||
before, err := details.SDKClient.Workspace(context.Background(), details.Workspace.ID)
|
||||
require.NoError(t, err)
|
||||
// Wait for stats to fully flush.
|
||||
details.FlushStats()
|
||||
require.Eventually(t, func() bool {
|
||||
details.FlushStats()
|
||||
ws, err := details.SDKClient.Workspace(context.Background(), details.Workspace.ID)
|
||||
assert.NoError(t, err)
|
||||
return ws.LastUsedAt.After(details.Workspace.LastUsedAt)
|
||||
}, testutil.WaitShort, testutil.IntervalMedium, "workspace LastUsedAt not updated when it should have been")
|
||||
after, err := details.SDKClient.Workspace(context.Background(), details.Workspace.ID)
|
||||
return assert.NoError(t, err) && after.LastUsedAt.After(before.LastUsedAt)
|
||||
}, testutil.WaitShort, testutil.IntervalMedium)
|
||||
}
|
||||
|
||||
// Except when it sometimes shouldn't (e.g. no access)
|
||||
// NOTE: Despite our efforts with the flush channel, this is inherently racy.
|
||||
// NOTE: Despite our efforts with the flush channel, this is inherently racy when used with
|
||||
// parallel tests on the same workspace/app.
|
||||
func assertWorkspaceLastUsedAtNotUpdated(t testing.TB, details *Details) {
|
||||
t.Helper()
|
||||
|
||||
details.FlushStats()
|
||||
ws, err := details.SDKClient.Workspace(context.Background(), details.Workspace.ID)
|
||||
require.NotNil(t, details.Workspace, "can't assert LastUsedAt on a nil workspace!")
|
||||
before, err := details.SDKClient.Workspace(context.Background(), details.Workspace.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, ws.LastUsedAt, details.Workspace.LastUsedAt, "workspace LastUsedAt updated when it should not have been")
|
||||
details.FlushStats()
|
||||
after, err := details.SDKClient.Workspace(context.Background(), details.Workspace.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, before.LastUsedAt, after.LastUsedAt, "workspace LastUsedAt updated when it should not have been")
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue