Skip to content

Commit 65db7a7

Browse files
johnstcnjohnstcn
authored
feat(coderd/database/dbtestutil): set default database timezone to non-UTC in unit tests (#9672)
- Adds dbtestutil.WithTimezone(tz) to allow setting the timezone for a test database. - Modifies our test database setup code to pick a consistently weird timezone for the database. - Adds the facility randtz.Name() to pick a random timezone which is consistent across subtests (via sync.Once). - Adds a linter rule to warn against setting the test database timezone to UTC.
1 parent 281faf9 commit 65db7a7

File tree

12 files changed

+1276
-137
lines changed

12 files changed

+1276
-137
lines changed

‎cli/clitest/golden.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
// make update-golden-files
2727
var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files")
2828

29-
var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?Z`)
29+
var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?(Z|[+-]\d+:\d+)`)
3030

3131
type CommandHelpCase struct {
3232
Name string

‎coderd/activitybump_internal_test.go

Lines changed: 126 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package coderd
22

33
import (
44
"database/sql"
5-
"runtime"
65
"testing"
76
"time"
87

@@ -22,6 +21,16 @@ import (
2221
func Test_ActivityBumpWorkspace(t *testing.T) {
2322
t.Parallel()
2423

24+
// We test the below in multiple timezones specifically
25+
// chosen to trigger timezone-related bugs.
26+
timezones := []string{
27+
"Asia/Kolkata", // No DST, positive fractional offset
28+
"Canada/Newfoundland", // DST, negative fractional offset
29+
"Europe/Paris", // DST, positive offset
30+
"US/Arizona", // No DST, negative offset
31+
"UTC", // Baseline
32+
}
33+
2534
for _, tt := range []struct {
2635
name string
2736
transition database.WorkspaceTransition
@@ -92,117 +101,124 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
92101
},
93102
} {
94103
tt := tt
95-
t.Run(tt.name, func(t *testing.T) {
96-
t.Parallel()
97-
98-
var (
99-
now = dbtime.Now()
100-
ctx = testutil.Context(t, testutil.WaitShort)
101-
log = slogtest.Make(t, nil)
102-
db, _ = dbtestutil.NewDB(t)
103-
org = dbgen.Organization(t, db, database.Organization{})
104-
user = dbgen.User(t, db, database.User{
105-
Status: database.UserStatusActive,
106-
})
107-
_ = dbgen.OrganizationMember(t, db, database.OrganizationMember{
108-
UserID: user.ID,
109-
OrganizationID: org.ID,
110-
})
111-
templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{
112-
OrganizationID: org.ID,
113-
CreatedBy: user.ID,
114-
})
115-
template = dbgen.Template(t, db, database.Template{
116-
OrganizationID: org.ID,
117-
ActiveVersionID: templateVersion.ID,
118-
CreatedBy: user.ID,
119-
})
120-
ws = dbgen.Workspace(t, db, database.Workspace{
121-
OwnerID: user.ID,
122-
OrganizationID: org.ID,
123-
TemplateID: template.ID,
124-
Ttl: sql.NullInt64{Valid: true, Int64: int64(tt.workspaceTTL)},
125-
})
126-
job = dbgen.ProvisionerJob(t, db, database.ProvisionerJob{
127-
OrganizationID: org.ID,
128-
CompletedAt: tt.jobCompletedAt,
104+
for _, tz := range timezones {
105+
tz := tz
106+
t.Run(tt.name+"/"+tz, func(t *testing.T) {
107+
t.Parallel()
108+
109+
var (
110+
now = dbtime.Now()
111+
ctx = testutil.Context(t, testutil.WaitShort)
112+
log = slogtest.Make(t, nil)
113+
db, _ = dbtestutil.NewDB(t, dbtestutil.WithTimezone(tz))
114+
org = dbgen.Organization(t, db, database.Organization{})
115+
user = dbgen.User(t, db, database.User{
116+
Status: database.UserStatusActive,
117+
})
118+
_ = dbgen.OrganizationMember(t, db, database.OrganizationMember{
119+
UserID: user.ID,
120+
OrganizationID: org.ID,
121+
})
122+
templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{
123+
OrganizationID: org.ID,
124+
CreatedBy: user.ID,
125+
})
126+
template = dbgen.Template(t, db, database.Template{
127+
OrganizationID: org.ID,
128+
ActiveVersionID: templateVersion.ID,
129+
CreatedBy: user.ID,
130+
})
131+
ws = dbgen.Workspace(t, db, database.Workspace{
132+
OwnerID: user.ID,
133+
OrganizationID: org.ID,
134+
TemplateID: template.ID,
135+
Ttl: sql.NullInt64{Valid: true, Int64: int64(tt.workspaceTTL)},
136+
})
137+
job = dbgen.ProvisionerJob(t, db, database.ProvisionerJob{
138+
OrganizationID: org.ID,
139+
CompletedAt: tt.jobCompletedAt,
140+
})
141+
_ = dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
142+
JobID: job.ID,
143+
})
144+
buildID = uuid.New()
145+
)
146+
147+
var buildNumber int32 = 1
148+
// Insert a number of previous workspace builds.
149+
for i := 0; i < 5; i++ {
150+
insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStart, buildNumber)
151+
buildNumber++
152+
insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStop, buildNumber)
153+
buildNumber++
154+
}
155+
156+
// dbgen.WorkspaceBuild automatically sets deadline to now+1 hour if not set
157+
var buildDeadline time.Time
158+
if tt.buildDeadlineOffset != nil {
159+
buildDeadline = now.Add(*tt.buildDeadlineOffset)
160+
}
161+
var maxDeadline time.Time
162+
if tt.maxDeadlineOffset != nil {
163+
maxDeadline = now.Add(*tt.maxDeadlineOffset)
164+
}
165+
err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{
166+
ID: buildID,
167+
CreatedAt: dbtime.Now(),
168+
UpdatedAt: dbtime.Now(),
169+
BuildNumber: buildNumber,
170+
InitiatorID: user.ID,
171+
Reason: database.BuildReasonInitiator,
172+
WorkspaceID: ws.ID,
173+
JobID: job.ID,
174+
TemplateVersionID: templateVersion.ID,
175+
Transition: tt.transition,
176+
Deadline: buildDeadline,
177+
MaxDeadline: maxDeadline,
129178
})
130-
_ = dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
131-
JobID: job.ID,
132-
})
133-
buildID = uuid.New()
134-
)
135-
136-
var buildNumber int32 = 1
137-
// Insert a number of previous workspace builds.
138-
for i := 0; i < 5; i++ {
139-
insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStart, buildNumber)
140-
buildNumber++
141-
insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStop, buildNumber)
142-
buildNumber++
143-
}
144-
145-
// dbgen.WorkspaceBuild automatically sets deadline to now+1 hour if not set
146-
var buildDeadline time.Time
147-
if tt.buildDeadlineOffset != nil {
148-
buildDeadline = now.Add(*tt.buildDeadlineOffset)
149-
}
150-
var maxDeadline time.Time
151-
if tt.maxDeadlineOffset != nil {
152-
maxDeadline = now.Add(*tt.maxDeadlineOffset)
153-
}
154-
err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{
155-
ID: buildID,
156-
CreatedAt: dbtime.Now(),
157-
UpdatedAt: dbtime.Now(),
158-
BuildNumber: buildNumber,
159-
InitiatorID: user.ID,
160-
Reason: database.BuildReasonInitiator,
161-
WorkspaceID: ws.ID,
162-
JobID: job.ID,
163-
TemplateVersionID: templateVersion.ID,
164-
Transition: tt.transition,
165-
Deadline: buildDeadline,
166-
MaxDeadline: maxDeadline,
167-
})
168-
require.NoError(t, err, "unexpected error inserting workspace build")
169-
bld, err := db.GetWorkspaceBuildByID(ctx, buildID)
170-
require.NoError(t, err, "unexpected error fetching inserted workspace build")
171-
172-
// Validate our initial state before bump
173-
require.Equal(t, tt.transition, bld.Transition, "unexpected transition before bump")
174-
require.Equal(t, tt.jobCompletedAt.Time.UTC(), job.CompletedAt.Time.UTC(), "unexpected job completed at before bump")
175-
require.Equal(t, buildDeadline.UTC(), bld.Deadline.UTC(), "unexpected build deadline before bump")
176-
require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump")
177-
require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump")
178-
179-
workaroundWindowsTimeResolution(t)
180-
181-
// Bump duration is measured from the time of the bump, so we measure from here.
182-
start := dbtime.Now()
183-
activityBumpWorkspace(ctx, log, db, bld.WorkspaceID)
184-
elapsed := time.Since(start)
185-
if elapsed > 15*time.Second {
186-
t.Logf("warning: activityBumpWorkspace took longer than 15 seconds: %s", elapsed)
187-
}
188-
// The actual bump could have happened anywhere in the elapsed time, so we
189-
// guess at the approximate time of the bump.
190-
approxBumpTime := start.Add(elapsed / 2)
191-
192-
// Validate our state after bump
193-
updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID)
194-
require.NoError(t, err, "unexpected error getting latest workspace build")
195-
if tt.expectedBump == 0 {
196-
require.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at")
197-
require.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline")
198-
} else {
179+
require.NoError(t, err, "unexpected error inserting workspace build")
180+
bld, err := db.GetWorkspaceBuildByID(ctx, buildID)
181+
require.NoError(t, err, "unexpected error fetching inserted workspace build")
182+
183+
// Validate our initial state before bump
184+
require.Equal(t, tt.transition, bld.Transition, "unexpected transition before bump")
185+
require.Equal(t, tt.jobCompletedAt.Time.UTC(), job.CompletedAt.Time.UTC(), "unexpected job completed at before bump")
186+
require.Equal(t, buildDeadline.UTC(), bld.Deadline.UTC(), "unexpected build deadline before bump")
187+
require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump")
188+
require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump")
189+
190+
// Wait a bit before bumping as dbtime is rounded to the nearest millisecond.
191+
// This should also hopefully be enough for Windows time resolution to register
192+
// a tick (win32 max timer resolution is apparently between 0.5 and 15.6ms)
193+
<-time.After(testutil.IntervalFast)
194+
195+
// Bump duration is measured from the time of the bump, so we measure from here.
196+
start := dbtime.Now()
197+
activityBumpWorkspace(ctx, log, db, bld.WorkspaceID)
198+
end := dbtime.Now()
199+
200+
// Validate our state after bump
201+
updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID)
202+
require.NoError(t, err, "unexpected error getting latest workspace build")
203+
require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "max_deadline should not have changed")
204+
if tt.expectedBump == 0 {
205+
require.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at")
206+
require.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline")
207+
return
208+
}
199209
require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at")
200-
expectedDeadline := approxBumpTime.Add(tt.expectedBump).UTC()
201-
// Note: if CI is especially slow, this test may fail. There is an internal 15-second
202-
// deadline in activityBumpWorkspace, so we allow the same window here.
203-
require.WithinDuration(t, expectedDeadline, updatedBuild.Deadline.UTC(), 15*time.Second, "unexpected deadline after bump")
204-
}
205-
})
210+
if tt.maxDeadlineOffset != nil {
211+
require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "new deadline must equal original max deadline")
212+
return
213+
}
214+
215+
// Assert that the bump occurred between start and end.
216+
expectedDeadlineStart := start.Add(tt.expectedBump)
217+
expectedDeadlineEnd := end.Add(tt.expectedBump)
218+
require.GreaterOrEqual(t, updatedBuild.Deadline, expectedDeadlineStart, "new deadline should be greater than or equal to start")
219+
require.LessOrEqual(t, updatedBuild.Deadline, expectedDeadlineEnd, "new deadline should be lesser than or equal to end")
220+
})
221+
}
206222
}
207223
}
208224

@@ -223,11 +239,3 @@ func insertPrevWorkspaceBuild(t *testing.T, db database.Store, orgID, tvID, work
223239
Transition: transition,
224240
})
225241
}
226-
227-
func workaroundWindowsTimeResolution(t *testing.T) {
228-
t.Helper()
229-
if runtime.GOOS == "windows" {
230-
t.Logf("workaround: sleeping for a short time to avoid time resolution issues on Windows")
231-
<-time.After(testutil.IntervalSlow)
232-
}
233-
}

‎coderd/activitybump_test.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
3030
// doesn't use template autostop requirements and instead edits the
3131
// max_deadline on the build directly in the database.
3232
setupActivityTest := func(t *testing.T, deadline ...time.Duration) (client *codersdk.Client, workspace codersdk.Workspace, assertBumped func(want bool)) {
33+
t.Helper()
3334
const ttl = time.Minute
3435
maxTTL := time.Duration(0)
3536
if len(deadline) > 0 {
@@ -120,6 +121,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
120121
_ = coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
121122

122123
return client, workspace, func(want bool) {
124+
t.Helper()
123125
if !want {
124126
// It is difficult to test the absence of a call in a non-racey
125127
// way. In general, it is difficult for the API to generate
@@ -134,24 +136,32 @@ func TestWorkspaceActivityBump(t *testing.T) {
134136
return
135137
}
136138

139+
var updatedAfter time.Time
137140
// The Deadline bump occurs asynchronously.
138141
require.Eventuallyf(t,
139142
func() bool {
140143
workspace, err = client.Workspace(ctx, workspace.ID)
141144
require.NoError(t, err)
142-
return workspace.LatestBuild.Deadline.Time != firstDeadline
145+
updatedAfter = dbtime.Now()
146+
if workspace.LatestBuild.Deadline.Time == firstDeadline {
147+
updatedAfter = time.Now()
148+
return false
149+
}
150+
return true
143151
},
144152
testutil.WaitLong, testutil.IntervalFast,
145153
"deadline %v never updated", firstDeadline,
146154
)
147155

156+
require.Greater(t, workspace.LatestBuild.Deadline.Time, updatedAfter)
157+
148158
// If the workspace has a max deadline, the deadline must not exceed
149159
// it.
150-
if maxTTL != 0 && dbtime.Now().Add(ttl).After(workspace.LatestBuild.MaxDeadline.Time) {
151-
require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
160+
if workspace.LatestBuild.MaxDeadline.Valid {
161+
require.LessOrEqual(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
152162
return
153163
}
154-
require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, 3*time.Second)
164+
require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, testutil.WaitShort)
155165
}
156166
}
157167

@@ -210,12 +220,6 @@ func TestWorkspaceActivityBump(t *testing.T) {
210220
require.NoError(t, err)
211221
_ = sshConn.Close()
212222

213-
assertBumped(true)
214-
215-
// Double check that the workspace build's deadline is equal to the
216-
// max deadline.
217-
workspace, err = client.Workspace(ctx, workspace.ID)
218-
require.NoError(t, err)
219-
require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
223+
assertBumped(true) // also asserts max ttl not exceeded
220224
})
221225
}

0 commit comments

Comments
 (0)