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.

cli/clitest/golden.go

@@ -26,7 +26,7 @@ import (
 // make update-golden-files
 var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files")
 
-var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?Z`)
+var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?(Z|[+-]\d+:\d+)`)
 
 type CommandHelpCase struct {
 	Name string

coderd/activitybump_internal_test.go

@@ -2,7 +2,6 @@ package coderd
 
 import (
 	"database/sql"
-	"runtime"
 	"testing"
 	"time"
 
@@ -22,6 +21,16 @@ import (
 func Test_ActivityBumpWorkspace(t *testing.T) {
 	t.Parallel()
 
+	// We test the below in multiple timezones specifically
+	// chosen to trigger timezone-related bugs.
+	timezones := []string{
+		"Asia/Kolkata",        // No DST, positive fractional offset
+		"Canada/Newfoundland", // DST, negative fractional offset
+		"Europe/Paris",        // DST, positive offset
+		"US/Arizona",          // No DST, negative offset
+		"UTC",                 // Baseline
+	}
+
 	for _, tt := range []struct {
 		name                string
 		transition          database.WorkspaceTransition
@@ -92,14 +101,16 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 		},
 	} {
 		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
+		for _, tz := range timezones {
+			tz := tz
+			t.Run(tt.name+"/"+tz, func(t *testing.T) {
 				t.Parallel()
 
 				var (
 					now   = dbtime.Now()
 					ctx   = testutil.Context(t, testutil.WaitShort)
 					log   = slogtest.Make(t, nil)
-				db, _ = dbtestutil.NewDB(t)
+					db, _ = dbtestutil.NewDB(t, dbtestutil.WithTimezone(tz))
 					org   = dbgen.Organization(t, db, database.Organization{})
 					user  = dbgen.User(t, db, database.User{
 						Status: database.UserStatusActive,
@@ -176,34 +187,39 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 				require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump")
 				require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump")
 
-			workaroundWindowsTimeResolution(t)
+				// Wait a bit before bumping as dbtime is rounded to the nearest millisecond.
+				// This should also hopefully be enough for Windows time resolution to register
+				// a tick (win32 max timer resolution is apparently between 0.5 and 15.6ms)
+				<-time.After(testutil.IntervalFast)
 
 				// Bump duration is measured from the time of the bump, so we measure from here.
 				start := dbtime.Now()
 				activityBumpWorkspace(ctx, log, db, bld.WorkspaceID)
-			elapsed := time.Since(start)
+				end := dbtime.Now()
-			if elapsed > 15*time.Second {
-				t.Logf("warning: activityBumpWorkspace took longer than 15 seconds: %s", elapsed)
-			}
-			// The actual bump could have happened anywhere in the elapsed time, so we
-			// guess at the approximate time of the bump.
-			approxBumpTime := start.Add(elapsed / 2)
 
 				// Validate our state after bump
 				updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID)
 				require.NoError(t, err, "unexpected error getting latest workspace build")
+				require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "max_deadline should not have changed")
 				if tt.expectedBump == 0 {
 					require.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at")
 					require.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline")
-			} else {
+					return
-				require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at")
-				expectedDeadline := approxBumpTime.Add(tt.expectedBump).UTC()
-				// Note: if CI is especially slow, this test may fail. There is an internal 15-second
-				// deadline in activityBumpWorkspace, so we allow the same window here.
-				require.WithinDuration(t, expectedDeadline, updatedBuild.Deadline.UTC(), 15*time.Second, "unexpected deadline after bump")
 				}
+				require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at")
+				if tt.maxDeadlineOffset != nil {
+					require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "new deadline must equal original max deadline")
+					return
+				}
+
+				// Assert that the bump occurred between start and end.
+				expectedDeadlineStart := start.Add(tt.expectedBump)
+				expectedDeadlineEnd := end.Add(tt.expectedBump)
+				require.GreaterOrEqual(t, updatedBuild.Deadline, expectedDeadlineStart, "new deadline should be greater than or equal to start")
+				require.LessOrEqual(t, updatedBuild.Deadline, expectedDeadlineEnd, "new deadline should be lesser than or equal to end")
 			})
 		}
+	}
 }
 
 func insertPrevWorkspaceBuild(t *testing.T, db database.Store, orgID, tvID, workspaceID uuid.UUID, transition database.WorkspaceTransition, buildNumber int32) {
@@ -223,11 +239,3 @@ func insertPrevWorkspaceBuild(t *testing.T, db database.Store, orgID, tvID, work
 		Transition:        transition,
 	})
 }
-
-func workaroundWindowsTimeResolution(t *testing.T) {
-	t.Helper()
-	if runtime.GOOS == "windows" {
-		t.Logf("workaround: sleeping for a short time to avoid time resolution issues on Windows")
-		<-time.After(testutil.IntervalSlow)
-	}
-}

coderd/activitybump_test.go

@@ -30,6 +30,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
 	// doesn't use template autostop requirements and instead edits the
 	// max_deadline on the build directly in the database.
 	setupActivityTest := func(t *testing.T, deadline ...time.Duration) (client *codersdk.Client, workspace codersdk.Workspace, assertBumped func(want bool)) {
+		t.Helper()
 		const ttl = time.Minute
 		maxTTL := time.Duration(0)
 		if len(deadline) > 0 {
@@ -120,6 +121,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
 		_ = coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
 
 		return client, workspace, func(want bool) {
+			t.Helper()
 			if !want {
 				// It is difficult to test the absence of a call in a non-racey
 				// way. In general, it is difficult for the API to generate
@@ -134,24 +136,32 @@ func TestWorkspaceActivityBump(t *testing.T) {
 				return
 			}
 
+			var updatedAfter time.Time
 			// The Deadline bump occurs asynchronously.
 			require.Eventuallyf(t,
 				func() bool {
 					workspace, err = client.Workspace(ctx, workspace.ID)
 					require.NoError(t, err)
-					return workspace.LatestBuild.Deadline.Time != firstDeadline
+					updatedAfter = dbtime.Now()
+					if workspace.LatestBuild.Deadline.Time == firstDeadline {
+						updatedAfter = time.Now()
+						return false
+					}
+					return true
 				},
 				testutil.WaitLong, testutil.IntervalFast,
 				"deadline %v never updated", firstDeadline,
 			)
 
+			require.Greater(t, workspace.LatestBuild.Deadline.Time, updatedAfter)
+
 			// If the workspace has a max deadline, the deadline must not exceed
 			// it.
-			if maxTTL != 0 && dbtime.Now().Add(ttl).After(workspace.LatestBuild.MaxDeadline.Time) {
+			if workspace.LatestBuild.MaxDeadline.Valid {
-				require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
+				require.LessOrEqual(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
 				return
 			}
-			require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, 3*time.Second)
+			require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, testutil.WaitShort)
 		}
 	}
 
@@ -210,12 +220,6 @@ func TestWorkspaceActivityBump(t *testing.T) {
 		require.NoError(t, err)
 		_ = sshConn.Close()
 
-		assertBumped(true)
+		assertBumped(true) // also asserts max ttl not exceeded
-
-		// Double check that the workspace build's deadline is equal to the
-		// max deadline.
-		workspace, err = client.Workspace(ctx, workspace.ID)
-		require.NoError(t, err)
-		require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
 	})
 }

coderd/database/dbtestutil/db.go

@@ -3,7 +3,10 @@ package dbtestutil
 import (
 	"context"
 	"database/sql"
+	"fmt"
+	"net/url"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -19,9 +22,27 @@ func WillUsePostgres() bool {
 	return os.Getenv("DB") != ""
 }
 
-func NewDB(t testing.TB) (database.Store, pubsub.Pubsub) {
+type options struct {
+	fixedTimezone string
+}
+
+type Option func(*options)
+
+// WithTimezone sets the database to the defined timezone.
+func WithTimezone(tz string) Option {
+	return func(o *options) {
+		o.fixedTimezone = tz
+	}
+}
+
+func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) {
 	t.Helper()
 
+	var o options
+	for _, opt := range opts {
+		opt(&o)
+	}
+
 	db := dbfake.New()
 	ps := pubsub.NewInMemory()
 	if WillUsePostgres() {
@@ -35,6 +56,19 @@ func NewDB(t testing.TB) (database.Store, pubsub.Pubsub) {
 			require.NoError(t, err)
 			t.Cleanup(closePg)
 		}
+
+		if o.fixedTimezone == "" {
+			// To make sure we find timezone-related issues, we set the timezone
+			// of the database to a non-UTC one.
+			// The below was picked due to the following properties:
+			// - It has a non-UTC offset
+			// - It has a fractional hour UTC offset
+			// - It includes a daylight savings time component
+			o.fixedTimezone = "Canada/Newfoundland"
+		}
+		dbName := dbNameFromConnectionURL(t, connectionURL)
+		setDBTimezone(t, connectionURL, dbName, o.fixedTimezone)
+
 		sqlDB, err := sql.Open("postgres", connectionURL)
 		require.NoError(t, err)
 		t.Cleanup(func() {
@@ -51,3 +85,28 @@ func NewDB(t testing.TB) (database.Store, pubsub.Pubsub) {
 
 	return db, ps
 }
+
+// setRandDBTimezone sets the timezone of the database to the given timezone.
+// Note that the updated timezone only comes into effect on reconnect, so we
+// create our own connection for this and close the DB after we're done.
+func setDBTimezone(t testing.TB, dbURL, dbname, tz string) {
+	t.Helper()
+
+	sqlDB, err := sql.Open("postgres", dbURL)
+	require.NoError(t, err)
+	defer func() {
+		_ = sqlDB.Close()
+	}()
+
+	// nolint: gosec // This unfortunately does not work with placeholders.
+	_, err = sqlDB.Exec(fmt.Sprintf("ALTER DATABASE %s SET TIMEZONE TO %q", dbname, tz))
+	require.NoError(t, err, "failed to set timezone for database")
+}
+
+// dbNameFromConnectionURL returns the database name from the given connection URL,
+// where connectionURL is of the form postgres://user:pass@host:port/dbname
+func dbNameFromConnectionURL(t testing.TB, connectionURL string) string {
+	u, err := url.Parse(connectionURL)
+	require.NoError(t, err)
+	return strings.TrimPrefix(u.Path, "/")
+}

coderd/database/queries.sql.go

@@ -19,10 +19,10 @@ const activityBumpWorkspace = `-- name: ActivityBumpWorkspace :exec
 WITH latest AS (
 	SELECT
 		workspace_builds.id::uuid AS build_id,
-		workspace_builds.deadline::timestamp AS build_deadline,
+		workspace_builds.deadline::timestamp with time zone AS build_deadline,
-		workspace_builds.max_deadline::timestamp AS build_max_deadline,
+		workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline,
 		workspace_builds.transition AS build_transition,
-		provisioner_jobs.completed_at::timestamp AS job_completed_at,
+		provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at,
 		(workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval
 	FROM workspace_builds
 	JOIN provisioner_jobs

coderd/database/queries/activitybump.sql

@@ -6,10 +6,10 @@
 WITH latest AS (
 	SELECT
 		workspace_builds.id::uuid AS build_id,
-		workspace_builds.deadline::timestamp AS build_deadline,
+		workspace_builds.deadline::timestamp with time zone AS build_deadline,
-		workspace_builds.max_deadline::timestamp AS build_max_deadline,
+		workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline,
 		workspace_builds.transition AS build_transition,
-		provisioner_jobs.completed_at::timestamp AS job_completed_at,
+		provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at,
 		(workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval
 	FROM workspace_builds
 	JOIN provisioner_jobs

coderd/workspaceagents_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/coder/coder/v2/agent"
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbtestutil"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -1184,8 +1185,12 @@ func TestWorkspaceAgent_LifecycleState(t *testing.T) {
 func TestWorkspaceAgent_Metadata(t *testing.T) {
 	t.Parallel()
 
+	// nolint:gocritic // https://github.com/coder/coder/issues/9682
+	db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC"))
 	client := coderdtest.New(t, &coderdtest.Options{
 		IncludeProvisionerDaemon: true,
+		Database:                 db,
+		Pubsub:                   ps,
 	})
 	user := coderdtest.CreateFirstUser(t, client)
 	authToken := uuid.NewString()

coderd/workspaces_test.go

@@ -1514,8 +1514,13 @@ func TestWorkspaceFilterManual(t *testing.T) {
 
 	t.Run("LastUsed", func(t *testing.T) {
 		t.Parallel()
+
+		// nolint:gocritic // https://github.com/coder/coder/issues/9682
+		db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC"))
 		client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
 			IncludeProvisionerDaemon: true,
+			Database:                 db,
+			Pubsub:                   ps,
 		})
 		user := coderdtest.CreateFirstUser(t, client)
 		authToken := uuid.NewString()

enterprise/coderd/templates_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/coder/coder/v2/coderd/audit"
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbtestutil"
 	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/cryptorand"
@@ -368,10 +369,14 @@ func TestTemplates(t *testing.T) {
 	t.Run("UpdateLastUsedAt", func(t *testing.T) {
 		t.Parallel()
 
+		// nolint:gocritic // https://github.com/coder/coder/issues/9682
+		db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC"))
 		ctx := testutil.Context(t, testutil.WaitMedium)
 		client, user := coderdenttest.New(t, &coderdenttest.Options{
 			Options: &coderdtest.Options{
 				IncludeProvisionerDaemon: true,
+				Database:                 db,
+				Pubsub:                   ps,
 			},
 			LicenseOptions: &coderdenttest.LicenseOptions{
 				Features: license.Features{

enterprise/coderd/workspaces_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/coder/coder/v2/coderd/autobuild"
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbtestutil"
 	agplschedule "github.com/coder/coder/v2/coderd/schedule"
 	"github.com/coder/coder/v2/coderd/schedule/cron"
 	"github.com/coder/coder/v2/coderd/util/ptr"
@@ -641,9 +642,14 @@ func TestWorkspacesFiltering(t *testing.T) {
 
 		dormantTTL := 24 * time.Hour
 
+		// nolint:gocritic // https://github.com/coder/coder/issues/9682
+		db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC"))
+
 		client, user := coderdenttest.New(t, &coderdenttest.Options{
 			Options: &coderdtest.Options{
 				IncludeProvisionerDaemon: true,
+				Database:                 db,
+				Pubsub:                   ps,
 			},
 			LicenseOptions: &coderdenttest.LicenseOptions{
 				Features: license.Features{

scripts/rules.go

@@ -395,3 +395,16 @@ func slogError(m dsl.Matcher) {
 		Where(m["name"].Const && m["value"].Type.Is("error") && !m["name"].Text.Matches(`^"internal_error"$`)).
 		Report(`Error should be logged using "slog.Error" instead.`)
 }
+
+// withTimezoneUTC ensures that we don't just sprinkle dbtestutil.WithTimezone("UTC") about
+// to work around real timezone bugs in our code.
+//
+//nolint:unused,deadcode,varnamelen
+func withTimezoneUTC(m dsl.Matcher) {
+	m.Match(
+		`dbtestutil.WithTimezone($tz)`,
+	).Where(
+		m["tz"].Text.Matches(`[uU][tT][cC]"$`),
+	).Report(`Setting database timezone to UTC may mask timezone-related bugs.`).
+		At(m["tz"])
+}