Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8ea5fb7

Browse files
authored
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.
1 parent5454f49 commit8ea5fb7

File tree

1 file changed

+66
-22
lines changed

1 file changed

+66
-22
lines changed

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 66 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
7070

7171
t.Run("SignedTokenQueryParameter",func(t*testing.T) {
7272
t.Parallel()
73+
7374
ifappHostIsPrimary {
7475
t.Skip("Tickets are not used for terminal requests on the primary.")
7576
}
@@ -101,8 +102,6 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
101102
t.Run("WorkspaceAppsProxyPath",func(t*testing.T) {
102103
t.Parallel()
103104

104-
appDetails:=setupProxyTest(t,nil)
105-
106105
t.Run("Disabled",func(t*testing.T) {
107106
t.Parallel()
108107

@@ -132,6 +131,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
132131
t.Skip("This test only applies when testing apps on the primary.")
133132
}
134133

134+
appDetails:=setupProxyTest(t,nil)
135135
unauthedClient:=appDetails.AppClient(t)
136136
unauthedClient.SetSessionToken("")
137137

@@ -148,7 +148,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
148148
require.NoError(t,err)
149149
require.True(t,loc.Query().Has("message"))
150150
require.True(t,loc.Query().Has("redirect"))
151-
assertWorkspaceLastUsedAtUpdated(t,appDetails)
151+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
152152
})
153153

154154
t.Run("LoginWithoutAuthOnProxy",func(t*testing.T) {
@@ -158,6 +158,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
158158
t.Skip("This test only applies when testing apps on workspace proxies.")
159159
}
160160

161+
appDetails:=setupProxyTest(t,nil)
161162
unauthedClient:=appDetails.AppClient(t)
162163
unauthedClient.SetSessionToken("")
163164

@@ -186,12 +187,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
186187
// request is getting stripped.
187188
require.Equal(t,u.Path,redirectURI.Path+"/")
188189
require.Equal(t,u.RawQuery,redirectURI.RawQuery)
189-
assertWorkspaceLastUsedAtUpdated(t,appDetails)
190+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
190191
})
191192

192193
t.Run("NoAccessShould404",func(t*testing.T) {
193194
t.Parallel()
194195

196+
appDetails:=setupProxyTest(t,nil)
195197
userClient,_:=coderdtest.CreateAnotherUser(t,appDetails.SDKClient,appDetails.FirstUser.OrganizationID,rbac.RoleMember())
196198
userAppClient:=appDetails.AppClient(t)
197199
userAppClient.SetSessionToken(userClient.SessionToken())
@@ -210,6 +212,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
210212
t.Run("RedirectsWithSlash",func(t*testing.T) {
211213
t.Parallel()
212214

215+
appDetails:=setupProxyTest(t,nil)
213216
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
214217
defercancel()
215218

@@ -226,6 +229,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
226229
t.Run("RedirectsWithQuery",func(t*testing.T) {
227230
t.Parallel()
228231

232+
appDetails:=setupProxyTest(t,nil)
229233
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
230234
defercancel()
231235

@@ -245,6 +249,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
245249
t.Run("Proxies",func(t*testing.T) {
246250
t.Parallel()
247251

252+
appDetails:=setupProxyTest(t,nil)
248253
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
249254
defercancel()
250255

@@ -333,6 +338,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
333338
t.Run("BlocksMe",func(t*testing.T) {
334339
t.Parallel()
335340

341+
appDetails:=setupProxyTest(t,nil)
336342
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
337343
defercancel()
338344

@@ -347,13 +353,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
347353
body,err:=io.ReadAll(resp.Body)
348354
require.NoError(t,err)
349355
require.Contains(t,string(body),"must be accessed with the full username, not @me")
350-
// TODO(cian): A blocked request should not count as workspace usage.
351-
// assertWorkspaceLastUsedAtNotUpdated(t, appDetails.AppClient(t), appDetails)
356+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
352357
})
353358

354359
t.Run("ForwardsIP",func(t*testing.T) {
355360
t.Parallel()
356361

362+
appDetails:=setupProxyTest(t,nil)
357363
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
358364
defercancel()
359365

@@ -373,6 +379,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
373379
t.Run("ProxyError",func(t*testing.T) {
374380
t.Parallel()
375381

382+
appDetails:=setupProxyTest(t,nil)
376383
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
377384
defercancel()
378385

@@ -388,6 +395,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
388395
t.Run("NoProxyPort",func(t*testing.T) {
389396
t.Parallel()
390397

398+
appDetails:=setupProxyTest(t,nil)
391399
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
392400
defercancel()
393401

@@ -397,7 +405,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
397405
// TODO(@deansheather): This should be 400. There's a todo in the
398406
// resolve request code to fix this.
399407
require.Equal(t,http.StatusInternalServerError,resp.StatusCode)
400-
assertWorkspaceLastUsedAtUpdated(t,appDetails)
408+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
401409
})
402410
})
403411

@@ -636,6 +644,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
636644
_=resp.Body.Close()
637645
require.Equal(t,http.StatusOK,resp.StatusCode)
638646
require.Equal(t,resp.Header.Get("X-Got-Host"),u.Host)
647+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
639648
})
640649

641650
t.Run("WorkspaceAppsProxySubdomainHostnamePrefix/Different",func(t*testing.T) {
@@ -686,6 +695,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
686695
require.NoError(t,err)
687696
_=resp.Body.Close()
688697
require.NotEqual(t,http.StatusOK,resp.StatusCode)
698+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
689699
})
690700

691701
// This test ensures that the subdomain handler does nothing if
@@ -754,11 +764,10 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
754764
t.Run("WorkspaceAppsProxySubdomain",func(t*testing.T) {
755765
t.Parallel()
756766

757-
appDetails:=setupProxyTest(t,nil)
758-
759767
t.Run("NoAccessShould401",func(t*testing.T) {
760768
t.Parallel()
761769

770+
appDetails:=setupProxyTest(t,nil)
762771
userClient,_:=coderdtest.CreateAnotherUser(t,appDetails.SDKClient,appDetails.FirstUser.OrganizationID,rbac.RoleMember())
763772
userAppClient:=appDetails.AppClient(t)
764773
userAppClient.SetSessionToken(userClient.SessionToken())
@@ -770,11 +779,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
770779
require.NoError(t,err)
771780
deferresp.Body.Close()
772781
require.Equal(t,http.StatusNotFound,resp.StatusCode)
782+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
773783
})
774784

775785
t.Run("RedirectsWithSlash",func(t*testing.T) {
776786
t.Parallel()
777787

788+
appDetails:=setupProxyTest(t,nil)
778789
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
779790
defercancel()
780791

@@ -789,11 +800,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
789800
loc,err:=resp.Location()
790801
require.NoError(t,err)
791802
require.Equal(t,appDetails.SubdomainAppURL(appDetails.Apps.Owner).Path,loc.Path)
803+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
792804
})
793805

794806
t.Run("RedirectsWithQuery",func(t*testing.T) {
795807
t.Parallel()
796808

809+
appDetails:=setupProxyTest(t,nil)
797810
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
798811
defercancel()
799812

@@ -807,11 +820,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
807820
loc,err:=resp.Location()
808821
require.NoError(t,err)
809822
require.Equal(t,appDetails.SubdomainAppURL(appDetails.Apps.Owner).RawQuery,loc.RawQuery)
823+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
810824
})
811825

812826
t.Run("Proxies",func(t*testing.T) {
813827
t.Parallel()
814828

829+
appDetails:=setupProxyTest(t,nil)
815830
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
816831
defercancel()
817832

@@ -848,6 +863,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
848863
require.NoError(t,err)
849864
require.Equal(t,proxyTestAppBody,string(body))
850865
require.Equal(t,http.StatusOK,resp.StatusCode)
866+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
851867
})
852868

853869
t.Run("ProxiesHTTPS",func(t*testing.T) {
@@ -892,11 +908,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
892908
require.NoError(t,err)
893909
require.Equal(t,proxyTestAppBody,string(body))
894910
require.Equal(t,http.StatusOK,resp.StatusCode)
911+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
895912
})
896913

897914
t.Run("ProxiesPort",func(t*testing.T) {
898915
t.Parallel()
899916

917+
appDetails:=setupProxyTest(t,nil)
900918
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
901919
defercancel()
902920

@@ -907,23 +925,27 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
907925
require.NoError(t,err)
908926
require.Equal(t,proxyTestAppBody,string(body))
909927
require.Equal(t,http.StatusOK,resp.StatusCode)
928+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
910929
})
911930

912931
t.Run("ProxyError",func(t*testing.T) {
913932
t.Parallel()
914933

934+
appDetails:=setupProxyTest(t,nil)
915935
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
916936
defercancel()
917937

918938
resp,err:=appDetails.AppClient(t).Request(ctx,http.MethodGet,appDetails.SubdomainAppURL(appDetails.Apps.Fake).String(),nil)
919939
require.NoError(t,err)
920940
deferresp.Body.Close()
921941
require.Equal(t,http.StatusBadGateway,resp.StatusCode)
942+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
922943
})
923944

924945
t.Run("ProxyPortMinimumError",func(t*testing.T) {
925946
t.Parallel()
926947

948+
appDetails:=setupProxyTest(t,nil)
927949
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
928950
defercancel()
929951

@@ -939,6 +961,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
939961
err=json.NewDecoder(resp.Body).Decode(&resBody)
940962
require.NoError(t,err)
941963
require.Contains(t,resBody.Message,"Coder reserves ports less than")
964+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
942965
})
943966

944967
t.Run("SuffixWildcardOK",func(t*testing.T) {
@@ -961,6 +984,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
961984
require.NoError(t,err)
962985
require.Equal(t,proxyTestAppBody,string(body))
963986
require.Equal(t,http.StatusOK,resp.StatusCode)
987+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
964988
})
965989

966990
t.Run("WildcardPortOK",func(t*testing.T) {
@@ -993,16 +1017,19 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
9931017
require.NoError(t,err)
9941018
require.Equal(t,proxyTestAppBody,string(body))
9951019
require.Equal(t,http.StatusOK,resp.StatusCode)
1020+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
9961021
})
9971022

9981023
t.Run("SuffixWildcardNotMatch",func(t*testing.T) {
9991024
t.Parallel()
10001025

1001-
appDetails:=setupProxyTest(t,&DeploymentOptions{
1002-
AppHost:"*-suffix.test.coder.com",
1003-
})
1004-
10051026
t.Run("NoSuffix",func(t*testing.T) {
1027+
t.Parallel()
1028+
1029+
appDetails:=setupProxyTest(t,&DeploymentOptions{
1030+
AppHost:"*-suffix.test.coder.com",
1031+
})
1032+
10061033
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
10071034
defercancel()
10081035

@@ -1020,11 +1047,16 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
10201047
// It's probably rendering the dashboard or a 404 page, so only
10211048
// ensure that the body doesn't match.
10221049
require.NotContains(t,string(body),proxyTestAppBody)
1050+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
10231051
})
10241052

10251053
t.Run("DifferentSuffix",func(t*testing.T) {
10261054
t.Parallel()
10271055

1056+
appDetails:=setupProxyTest(t,&DeploymentOptions{
1057+
AppHost:"*-suffix.test.coder.com",
1058+
})
1059+
10281060
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
10291061
defercancel()
10301062

@@ -1042,6 +1074,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
10421074
// It's probably rendering the dashboard, so only ensure that the body
10431075
// doesn't match.
10441076
require.NotContains(t,string(body),proxyTestAppBody)
1077+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
10451078
})
10461079
})
10471080
})
@@ -1062,6 +1095,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
10621095
require.NoError(t,err)
10631096
deferresp.Body.Close()
10641097
require.Equal(t,http.StatusNotFound,resp.StatusCode)
1098+
assertWorkspaceLastUsedAtNotUpdated(t,appDetails)
10651099
})
10661100

10671101
t.Run("AuthenticatedOK",func(t*testing.T) {
@@ -1090,6 +1124,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
10901124
require.NoError(t,err)
10911125
deferresp.Body.Close()
10921126
require.Equal(t,http.StatusOK,resp.StatusCode)
1127+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
10931128
})
10941129

10951130
t.Run("PublicOK",func(t*testing.T) {
@@ -1117,6 +1152,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
11171152
require.NoError(t,err)
11181153
deferresp.Body.Close()
11191154
require.Equal(t,http.StatusOK,resp.StatusCode)
1155+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
11201156
})
11211157

11221158
t.Run("HTTPS",func(t*testing.T) {
@@ -1145,6 +1181,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
11451181
require.NoError(t,err)
11461182
deferresp.Body.Close()
11471183
require.Equal(t,http.StatusOK,resp.StatusCode)
1184+
assertWorkspaceLastUsedAtUpdated(t,appDetails)
11481185
})
11491186
})
11501187

@@ -1719,26 +1756,33 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
17191756
}
17201757

17211758
// Accessing an app should update the workspace's LastUsedAt.
1722-
// NOTE: Despite our efforts with the flush channel, this is inherently racy.
1759+
// NOTE: Despite our efforts with the flush channel, this is inherently racy when used with
1760+
// parallel tests on the same workspace/app.
17231761
funcassertWorkspaceLastUsedAtUpdated(t testing.TB,details*Details) {
17241762
t.Helper()
17251763

1764+
require.NotNil(t,details.Workspace,"can't assert LastUsedAt on a nil workspace!")
1765+
before,err:=details.SDKClient.Workspace(context.Background(),details.Workspace.ID)
1766+
require.NoError(t,err)
17261767
// Wait for stats to fully flush.
1768+
details.FlushStats()
17271769
require.Eventually(t,func()bool {
1728-
details.FlushStats()
1729-
ws,err:=details.SDKClient.Workspace(context.Background(),details.Workspace.ID)
1730-
assert.NoError(t,err)
1731-
returnws.LastUsedAt.After(details.Workspace.LastUsedAt)
1732-
},testutil.WaitShort,testutil.IntervalMedium,"workspace LastUsedAt not updated when it should have been")
1770+
after,err:=details.SDKClient.Workspace(context.Background(),details.Workspace.ID)
1771+
returnassert.NoError(t,err)&&after.LastUsedAt.After(before.LastUsedAt)
1772+
},testutil.WaitShort,testutil.IntervalMedium)
17331773
}
17341774

17351775
// Except when it sometimes shouldn't (e.g. no access)
1736-
// NOTE: Despite our efforts with the flush channel, this is inherently racy.
1776+
// NOTE: Despite our efforts with the flush channel, this is inherently racy when used with
1777+
// parallel tests on the same workspace/app.
17371778
funcassertWorkspaceLastUsedAtNotUpdated(t testing.TB,details*Details) {
17381779
t.Helper()
17391780

1781+
require.NotNil(t,details.Workspace,"can't assert LastUsedAt on a nil workspace!")
1782+
before,err:=details.SDKClient.Workspace(context.Background(),details.Workspace.ID)
1783+
require.NoError(t,err)
17401784
details.FlushStats()
1741-
ws,err:=details.SDKClient.Workspace(context.Background(),details.Workspace.ID)
1785+
after,err:=details.SDKClient.Workspace(context.Background(),details.Workspace.ID)
17421786
require.NoError(t,err)
1743-
require.Equal(t,ws.LastUsedAt,details.Workspace.LastUsedAt,"workspace LastUsedAt updated when it should not have been")
1787+
require.Equal(t,before.LastUsedAt,after.LastUsedAt,"workspace LastUsedAt updated when it should not have been")
17441788
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp