Skip to content

Commit 1e6ea61

Browse files
f0sseldeansheather
f0ssel
and
deansheather
authored
fix: pass in time parameter to prevent flakes (#11023)
Co-authored-by: Dean Sheather <dean@deansheather.com>
1 parent a42b6c1 commit 1e6ea61

File tree

2 files changed

+53
-13
lines changed

2 files changed

+53
-13
lines changed

‎coderd/insights.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func (api *API) insightsUserActivity(rw http.ResponseWriter, r *http.Request) {
8989
return
9090
}
9191

92-
startTime, endTime, ok := parseInsightsStartAndEndTime(ctx, rw, startTimeString, endTimeString)
92+
startTime, endTime, ok := parseInsightsStartAndEndTime(ctx, rw, time.Now(), startTimeString, endTimeString)
9393
if !ok {
9494
return
9595
}
@@ -176,7 +176,7 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) {
176176
return
177177
}
178178

179-
startTime, endTime, ok := parseInsightsStartAndEndTime(ctx, rw, startTimeString, endTimeString)
179+
startTime, endTime, ok := parseInsightsStartAndEndTime(ctx, rw, time.Now(), startTimeString, endTimeString)
180180
if !ok {
181181
return
182182
}
@@ -268,7 +268,7 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) {
268268
return
269269
}
270270

271-
startTime, endTime, ok := parseInsightsStartAndEndTime(ctx, rw, startTimeString, endTimeString)
271+
startTime, endTime, ok := parseInsightsStartAndEndTime(ctx, rw, time.Now(), startTimeString, endTimeString)
272272
if !ok {
273273
return
274274
}
@@ -539,9 +539,7 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage
539539
// time are not zero and that the end time is not before the start time. The
540540
// clock must be set to 00:00:00, except for "today", where end time is allowed
541541
// to provide the hour of the day (e.g. 14:00:00).
542-
func parseInsightsStartAndEndTime(ctx context.Context, rw http.ResponseWriter, startTimeString, endTimeString string) (startTime, endTime time.Time, ok bool) {
543-
now := time.Now()
544-
542+
func parseInsightsStartAndEndTime(ctx context.Context, rw http.ResponseWriter, now time.Time, startTimeString, endTimeString string) (startTime, endTime time.Time, ok bool) {
545543
for _, qp := range []struct {
546544
name, value string
547545
dest *time.Time
@@ -563,6 +561,9 @@ func parseInsightsStartAndEndTime(ctx context.Context, rw http.ResponseWriter, s
563561
return time.Time{}, time.Time{}, false
564562
}
565563

564+
// Change now to the same timezone as the parsed time.
565+
now := now.In(t.Location())
566+
566567
if t.IsZero() {
567568
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
568569
Message: "Query parameter has invalid value.",
@@ -604,7 +605,7 @@ func parseInsightsStartAndEndTime(ctx context.Context, rw http.ResponseWriter, s
604605
Validations: []codersdk.ValidationError{
605606
{
606607
Field: qp.name,
607-
Detail: fmt.Sprintf("Query param %q must have the clock set to 00:00:00", qp.name),
608+
Detail: fmt.Sprintf("Query param %q must have the clock set to 00:00:00, got %s", qp.name, qp.value),
608609
},
609610
},
610611
})
@@ -615,7 +616,7 @@ func parseInsightsStartAndEndTime(ctx context.Context, rw http.ResponseWriter, s
615616
Validations: []codersdk.ValidationError{
616617
{
617618
Field: qp.name,
618-
Detail: fmt.Sprintf("Query param %q must have the clock set to %02d:00:00", qp.name, h),
619+
Detail: fmt.Sprintf("Query param %q must have the clock set to %02d:00:00, got %s", qp.name, h, qp.value),
619620
},
620621
},
621622
})

‎coderd/insights_internal_test.go

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@ import (
1515
func Test_parseInsightsStartAndEndTime(t *testing.T) {
1616
t.Parallel()
1717

18+
t.Logf("machine location: %s", time.Now().Location())
1819
layout := insightsTimeLayout
1920
now := time.Now().UTC()
21+
t.Logf("now: %s", now)
22+
t.Logf("now location: %s", now.Location())
2023
y, m, d := now.Date()
2124
today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC)
25+
t.Logf("today: %s", today)
2226
thisHour := time.Date(y, m, d, now.Hour(), 0, 0, 0, time.UTC)
27+
t.Logf("thisHour: %s", thisHour)
2328
thisHourRoundUp := thisHour.Add(time.Hour)
29+
t.Logf("thisHourRoundUp: %s", thisHourRoundUp)
2430

2531
helsinki, err := time.LoadLocation("Europe/Helsinki")
2632
require.NoError(t, err)
@@ -36,6 +42,16 @@ func Test_parseInsightsStartAndEndTime(t *testing.T) {
3642
wantEndTime time.Time
3743
wantOk bool
3844
}{
45+
{
46+
name: "Same",
47+
args: args{
48+
startTime: "2023-07-10T00:00:00Z",
49+
endTime: "2023-07-10T00:00:00Z",
50+
},
51+
wantStartTime: time.Date(2023, 7, 10, 0, 0, 0, 0, time.UTC),
52+
wantEndTime: time.Date(2023, 7, 10, 0, 0, 0, 0, time.UTC),
53+
wantOk: true,
54+
},
3955
{
4056
name: "Week",
4157
args: args{
@@ -132,13 +148,21 @@ func Test_parseInsightsStartAndEndTime(t *testing.T) {
132148
t.Run(tt.name, func(t *testing.T) {
133149
t.Parallel()
134150

151+
t.Log("startTime: ", tt.args.startTime)
152+
t.Log("endTime: ", tt.args.endTime)
153+
if tt.wantOk {
154+
t.Log("wantStartTime: ", tt.wantStartTime)
155+
t.Log("wantEndTime: ", tt.wantEndTime)
156+
}
157+
135158
rw := httptest.NewRecorder()
136-
gotStartTime, gotEndTime, gotOk := parseInsightsStartAndEndTime(context.Background(), rw, tt.args.startTime, tt.args.endTime)
159+
gotStartTime, gotEndTime, gotOk := parseInsightsStartAndEndTime(context.Background(), rw, now, tt.args.startTime, tt.args.endTime)
137160

138161
if !assert.Equal(t, tt.wantOk, gotOk) {
139162
//nolint:bodyclose
140163
t.Log("Status: ", rw.Result().StatusCode)
141164
t.Log("Body: ", rw.Body.String())
165+
return
142166
}
143167
// assert.Equal is unable to test time equality with different
144168
// (but same) locations because the *time.Location names differ
@@ -158,7 +182,7 @@ func Test_parseInsightsInterval_week(t *testing.T) {
158182
sydneyLoc, err := time.LoadLocation("Australia/Sydney") // Random location
159183
require.NoError(t, err)
160184

161-
now := time.Now()
185+
now := time.Now().In(sydneyLoc)
162186
t.Logf("now: %s", now)
163187

164188
y, m, d := now.Date()
@@ -206,7 +230,7 @@ func Test_parseInsightsInterval_week(t *testing.T) {
206230
name: "6 days are acceptable",
207231
args: args{
208232
startTime: sixDaysAgo.Format(layout),
209-
endTime: thisHour.Format(layout),
233+
endTime: stripTime(thisHour).Format(layout),
210234
},
211235
wantOk: true,
212236
},
@@ -222,18 +246,22 @@ func Test_parseInsightsInterval_week(t *testing.T) {
222246
name: "9 days (7 + 2) are not acceptable",
223247
args: args{
224248
startTime: nineDaysAgo.Format(layout),
225-
endTime: thisHour.Format(layout),
249+
endTime: stripTime(thisHour).Format(layout),
226250
},
227251
wantOk: false,
228252
},
229253
}
230254
for _, tt := range tests {
231255
tt := tt
256+
232257
t.Run(tt.name, func(t *testing.T) {
233258
t.Parallel()
234259

260+
t.Log("startTime: ", tt.args.startTime)
261+
t.Log("endTime: ", tt.args.endTime)
262+
235263
rw := httptest.NewRecorder()
236-
startTime, endTime, ok := parseInsightsStartAndEndTime(context.Background(), rw, tt.args.startTime, tt.args.endTime)
264+
startTime, endTime, ok := parseInsightsStartAndEndTime(context.Background(), rw, now, tt.args.startTime, tt.args.endTime)
237265
if !ok {
238266
//nolint:bodyclose
239267
t.Log("Status: ", rw.Result().StatusCode)
@@ -246,6 +274,7 @@ func Test_parseInsightsInterval_week(t *testing.T) {
246274
//nolint:bodyclose
247275
t.Log("Status: ", rw.Result().StatusCode)
248276
t.Log("Body: ", rw.Body.String())
277+
return
249278
}
250279
if tt.wantOk {
251280
assert.Equal(t, codersdk.InsightsReportIntervalWeek, parsedInterval)
@@ -296,10 +325,20 @@ func TestLastReportIntervalHasAtLeastSixDays(t *testing.T) {
296325
tc := tc
297326
t.Run(tc.name, func(t *testing.T) {
298327
t.Parallel()
328+
329+
t.Log("startTime: ", tc.startTime)
330+
t.Log("endTime: ", tc.endTime)
331+
299332
result := lastReportIntervalHasAtLeastSixDays(tc.startTime, tc.endTime)
300333
if result != tc.expected {
301334
t.Errorf("Expected %v, but got %v for start time %v and end time %v", tc.expected, result, tc.startTime, tc.endTime)
302335
}
303336
})
304337
}
305338
}
339+
340+
// stripTime strips the time from a time.Time value, but keeps the date and TZ.
341+
func stripTime(t time.Time) time.Time {
342+
y, m, d := t.Date()
343+
return time.Date(y, m, d, 0, 0, 0, 0, t.Location())
344+
}

0 commit comments

Comments
 (0)