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

Commitdae1903

Browse files
test: fix TestCache_DeploymentStats flake (#19683)
Closescoder/internal#961Likely the same deal as in#19599, the body of `require.Eventually` now fires immediately, when it used to fire after 250ms (the interval). Presumably, the deployment stats become ready before the vs code session count gets incremented. This was never an issue with the 250ms delay, as this flake has only cropped up after the testify version bump.We'll fix the issue by making it possible to wait for a full metrics cache refresh, i.e. removing `require.Eventually` in this test altogether.
1 parente12b621 commitdae1903

File tree

3 files changed

+84
-77
lines changed

3 files changed

+84
-77
lines changed

‎coderd/coderd.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ func New(options *Options) *API {
459459
metricsCache:=metricscache.New(
460460
options.Database,
461461
options.Logger.Named("metrics_cache"),
462-
options.Clock,
462+
quartz.NewReal(),
463463
metricscache.Intervals{
464464
TemplateBuildTimes:options.MetricsCacheRefreshInterval,
465465
DeploymentStats:options.AgentStatsRefreshInterval,

‎coderd/metricscache/metricscache.go‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -181,35 +181,32 @@ func (c *Cache) refreshDeploymentStats(ctx context.Context) error {
181181

182182
func (c*Cache)run(ctx context.Context,namestring,interval time.Duration,refreshfunc(context.Context)error) {
183183
logger:=c.log.With(slog.F("name",name),slog.F("interval",interval))
184-
ticker:=time.NewTicker(interval)
185-
deferticker.Stop()
186184

187-
for {
185+
tickerFunc:=func()error {
186+
start:=c.clock.Now()
188187
forr:=retry.New(time.Millisecond*100,time.Minute);r.Wait(ctx); {
189-
start:=time.Now()
190188
err:=refresh(ctx)
191189
iferr!=nil {
192190
ifctx.Err()!=nil {
193-
return
191+
returnnil
194192
}
195193
ifxerrors.Is(err,sql.ErrNoRows) {
196194
break
197195
}
198196
logger.Error(ctx,"refresh metrics failed",slog.Error(err))
199197
continue
200198
}
201-
logger.Debug(ctx,"metrics refreshed",slog.F("took",time.Since(start)))
199+
logger.Debug(ctx,"metrics refreshed",slog.F("took",c.clock.Since(start)))
202200
break
203201
}
204-
205-
select {
206-
case<-ticker.C:
207-
case<-c.done:
208-
return
209-
case<-ctx.Done():
210-
return
211-
}
202+
returnnil
212203
}
204+
205+
// Call once immediately before starting ticker
206+
_=tickerFunc()
207+
208+
tkr:=c.clock.TickerFunc(ctx,interval,tickerFunc,"metricscache",name)
209+
_=tkr.Wait()
213210
}
214211

215212
func (c*Cache)Close()error {

‎coderd/metricscache/metricscache_test.go‎

Lines changed: 72 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,20 @@ func newMetricsCache(t *testing.T, log slog.Logger, clock quartz.Clock, interval
4949

5050
funcTestCache_TemplateWorkspaceOwners(t*testing.T) {
5151
t.Parallel()
52-
var ()
5352

5453
var (
55-
log=testutil.Logger(t)
56-
clock=quartz.NewReal()
57-
cache,db=newMetricsCache(t,log,clock, metricscache.Intervals{
58-
TemplateBuildTimes:testutil.IntervalFast,
59-
},false)
54+
ctx=testutil.Context(t,testutil.WaitShort)
55+
log=testutil.Logger(t)
56+
clock=quartz.NewMock(t)
6057
)
6158

59+
trapTickerFunc:=clock.Trap().TickerFunc("metricscache")
60+
defertrapTickerFunc.Close()
61+
62+
cache,db:=newMetricsCache(t,log,clock, metricscache.Intervals{
63+
TemplateBuildTimes:time.Minute,
64+
},false)
65+
6266
org:=dbgen.Organization(t,db, database.Organization{})
6367
user1:=dbgen.User(t,db, database.User{})
6468
user2:=dbgen.User(t,db, database.User{})
@@ -67,38 +71,38 @@ func TestCache_TemplateWorkspaceOwners(t *testing.T) {
6771
Provisioner:database.ProvisionerTypeEcho,
6872
CreatedBy:user1.ID,
6973
})
70-
require.Eventuallyf(t,func()bool {
71-
count,ok:=cache.TemplateWorkspaceOwners(template.ID)
72-
returnok&&count==0
73-
},testutil.WaitShort,testutil.IntervalMedium,
74-
"TemplateWorkspaceOwners never populated 0 owners",
75-
)
74+
75+
// Wait for both ticker functions to be created (template build times and deployment stats)
76+
trapTickerFunc.MustWait(ctx).MustRelease(ctx)
77+
trapTickerFunc.MustWait(ctx).MustRelease(ctx)
78+
79+
clock.Advance(time.Minute).MustWait(ctx)
80+
81+
count,ok:=cache.TemplateWorkspaceOwners(template.ID)
82+
require.True(t,ok,"TemplateWorkspaceOwners should be populated")
83+
require.Equal(t,0,count,"should have 0 owners initially")
7684

7785
dbgen.Workspace(t,db, database.WorkspaceTable{
7886
OrganizationID:org.ID,
7987
TemplateID:template.ID,
8088
OwnerID:user1.ID,
8189
})
8290

83-
require.Eventuallyf(t,func()bool {
84-
count,_:=cache.TemplateWorkspaceOwners(template.ID)
85-
returncount==1
86-
},testutil.WaitShort,testutil.IntervalMedium,
87-
"TemplateWorkspaceOwners never populated 1 owner",
88-
)
91+
clock.Advance(time.Minute).MustWait(ctx)
92+
93+
count,_=cache.TemplateWorkspaceOwners(template.ID)
94+
require.Equal(t,1,count,"should have 1 owner after adding workspace")
8995

9096
workspace2:=dbgen.Workspace(t,db, database.WorkspaceTable{
9197
OrganizationID:org.ID,
9298
TemplateID:template.ID,
9399
OwnerID:user2.ID,
94100
})
95101

96-
require.Eventuallyf(t,func()bool {
97-
count,_:=cache.TemplateWorkspaceOwners(template.ID)
98-
returncount==2
99-
},testutil.WaitShort,testutil.IntervalMedium,
100-
"TemplateWorkspaceOwners never populated 2 owners",
101-
)
102+
clock.Advance(time.Minute).MustWait(ctx)
103+
104+
count,_=cache.TemplateWorkspaceOwners(template.ID)
105+
require.Equal(t,2,count,"should have 2 owners after adding second workspace")
102106

103107
// 3rd workspace should not be counted since we have the same owner as workspace2.
104108
dbgen.Workspace(t,db, database.WorkspaceTable{
@@ -112,12 +116,10 @@ func TestCache_TemplateWorkspaceOwners(t *testing.T) {
112116
Deleted:true,
113117
})
114118

115-
require.Eventuallyf(t,func()bool {
116-
count,_:=cache.TemplateWorkspaceOwners(template.ID)
117-
returncount==1
118-
},testutil.WaitShort,testutil.IntervalMedium,
119-
"TemplateWorkspaceOwners never populated 1 owner after delete",
120-
)
119+
clock.Advance(time.Minute).MustWait(ctx)
120+
121+
count,_=cache.TemplateWorkspaceOwners(template.ID)
122+
require.Equal(t,1,count,"should have 1 owner after deleting workspace")
121123
}
122124

123125
funcclockTime(t time.Time,hour,minute,secint) time.Time {
@@ -206,15 +208,20 @@ func TestCache_BuildTime(t *testing.T) {
206208
t.Parallel()
207209

208210
var (
209-
log=testutil.Logger(t)
210-
clock=quartz.NewMock(t)
211-
cache,db=newMetricsCache(t,log,clock, metricscache.Intervals{
212-
TemplateBuildTimes:testutil.IntervalFast,
213-
},false)
211+
ctx=testutil.Context(t,testutil.WaitShort)
212+
log=testutil.Logger(t)
213+
clock=quartz.NewMock(t)
214214
)
215215

216216
clock.Set(someDay)
217217

218+
trapTickerFunc:=clock.Trap().TickerFunc("metricscache")
219+
220+
defertrapTickerFunc.Close()
221+
cache,db:=newMetricsCache(t,log,clock, metricscache.Intervals{
222+
TemplateBuildTimes:time.Minute,
223+
},false)
224+
218225
org:=dbgen.Organization(t,db, database.Organization{})
219226
user:=dbgen.User(t,db, database.User{})
220227

@@ -257,17 +264,19 @@ func TestCache_BuildTime(t *testing.T) {
257264
})
258265
}
259266

267+
// Wait for both ticker functions to be created (template build times and deployment stats)
268+
trapTickerFunc.MustWait(ctx).MustRelease(ctx)
269+
trapTickerFunc.MustWait(ctx).MustRelease(ctx)
270+
271+
clock.Advance(time.Minute).MustWait(ctx)
272+
260273
iftt.want.loads {
261274
wantTransition:=codersdk.WorkspaceTransition(tt.args.transition)
262-
require.Eventuallyf(t,func()bool {
263-
stats:=cache.TemplateBuildTimeStats(template.ID)
264-
ts:=stats[wantTransition]
265-
returnts.P50!=nil&&*ts.P50==tt.want.buildTimeMs
266-
},testutil.WaitLong,testutil.IntervalMedium,
267-
"P50 never reached expected value for %v",wantTransition,
268-
)
269-
270275
gotStats:=cache.TemplateBuildTimeStats(template.ID)
276+
ts:=gotStats[wantTransition]
277+
require.NotNil(t,ts.P50,"P50 should be set for %v",wantTransition)
278+
require.Equal(t,tt.want.buildTimeMs,*ts.P50,"P50 should match expected value for %v",wantTransition)
279+
271280
fortransition,ts:=rangegotStats {
272281
iftransition==wantTransition {
273282
// Checked above
@@ -276,14 +285,8 @@ func TestCache_BuildTime(t *testing.T) {
276285
require.Empty(t,ts,"%v",transition)
277286
}
278287
}else {
279-
varstats codersdk.TemplateBuildTimeStats
280-
require.Never(t,func()bool {
281-
stats=cache.TemplateBuildTimeStats(template.ID)
282-
requireBuildTimeStatsEmpty(t,stats)
283-
returnt.Failed()
284-
},testutil.WaitShort/2,testutil.IntervalMedium,
285-
"BuildTimeStats populated",stats,
286-
)
288+
stats:=cache.TemplateBuildTimeStats(template.ID)
289+
requireBuildTimeStatsEmpty(t,stats)
287290
}
288291
})
289292
}
@@ -293,13 +296,18 @@ func TestCache_DeploymentStats(t *testing.T) {
293296
t.Parallel()
294297

295298
var (
296-
log=testutil.Logger(t)
297-
clock=quartz.NewMock(t)
298-
cache,db=newMetricsCache(t,log,clock, metricscache.Intervals{
299-
DeploymentStats:testutil.IntervalFast,
300-
},false)
299+
ctx=testutil.Context(t,testutil.WaitShort)
300+
log=testutil.Logger(t)
301+
clock=quartz.NewMock(t)
301302
)
302303

304+
tickerTrap:=clock.Trap().TickerFunc("metricscache")
305+
defertickerTrap.Close()
306+
307+
cache,db:=newMetricsCache(t,log,clock, metricscache.Intervals{
308+
DeploymentStats:time.Minute,
309+
},false)
310+
303311
err:=db.InsertWorkspaceAgentStats(context.Background(), database.InsertWorkspaceAgentStatsParams{
304312
ID: []uuid.UUID{uuid.New()},
305313
CreatedAt: []time.Time{clock.Now()},
@@ -323,11 +331,13 @@ func TestCache_DeploymentStats(t *testing.T) {
323331
})
324332
require.NoError(t,err)
325333

326-
varstat codersdk.DeploymentStats
327-
require.Eventually(t,func()bool {
328-
varokbool
329-
stat,ok=cache.DeploymentStats()
330-
returnok
331-
},testutil.WaitLong,testutil.IntervalMedium)
334+
// Wait for both ticker functions to be created (template build times and deployment stats)
335+
tickerTrap.MustWait(ctx).MustRelease(ctx)
336+
tickerTrap.MustWait(ctx).MustRelease(ctx)
337+
338+
clock.Advance(time.Minute).MustWait(ctx)
339+
340+
stat,ok:=cache.DeploymentStats()
341+
require.True(t,ok,"cache should be populated after refresh")
332342
require.Equal(t,int64(1),stat.SessionCount.VSCode)
333343
}

0 commit comments

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