fix: fix TestExecutorAutostartSkipsWhenNoProvisionersAvailable flake, part 2 (#19649)

cstyan · web-flow · commit 6574299fccc4 · 2025-09-01T08:52:34.000-07:00
This test still flakes occasionally, see coder/internal#954 (comment) The cause appears to be related to the assignment of `time.Now()` as the `LastSeenAt` time when creating a provisioner which can flake with the calculated scheduled next autostart and the code to set then `require.Eventually` the updated provisioner LastSeenAt. Instead we should simply calculate all time values for the stale portion of the test based on the provisioners LastSeenAt value to avoid such issues. Signed-off-by: Callum Styan <callumstyan@gmail.com>
diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"database/sql"
 	"errors"
-	"sync"
 	"testing"
 	"time"
 
@@ -1724,34 +1723,27 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) {
 	p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags)
 	require.NoError(t, err, "Error getting provisioner for workspace")
 
-	var wg sync.WaitGroup
-	wg.Add(2)
+	// We're going to use an artificial next scheduled autostart time, as opposed to calculating it via sched.Next, since
+	// we want to assert/require specific behavior here around the provisioner being stale, and therefore we need to be
+	// able to give the provisioner(s) specific `LastSeenAt` times while dealing with the contraint that we cannot set
+	// that value to some time in the past (relative to it's current value).
+	next := p.LastSeenAt.Time.Add(5 * time.Minute)
+	staleTime := next.Add(-(provisionerdserver.StaleInterval + time.Second))
+	coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, staleTime)
 
-	next := sched.Next(workspace.LatestBuild.CreatedAt)
-	go func() {
-		defer wg.Done()
-		// Ensure the provisioner is stale
-		staleTime := next.Add(-(provisionerdserver.StaleInterval * 2))
-		coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, staleTime)
-		p, err = coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags)
-		assert.NoError(t, err, "Error getting provisioner for workspace")
-		assert.Eventually(t, func() bool { return p.LastSeenAt.Time.UnixNano() == staleTime.UnixNano() }, testutil.WaitMedium, testutil.IntervalFast)
-	}()
-
-	go func() {
-		defer wg.Done()
-		// Ensure the provisioner is gone or stale before triggering the autobuild
-		coderdtest.MustWaitForProvisionersUnavailable(t, db, workspace, provisionerDaemonTags, next)
-		// Trigger autobuild
-		tickCh <- next
-	}()
+	// Require that the provisioners LastSeenAt has been updated to the expected time.
+	p, err = coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags)
+	require.NoError(t, err, "Error getting provisioner for workspace")
+	// This assertion *may* no longer need to be `Eventually`.
+	require.Eventually(t, func() bool { return p.LastSeenAt.Time.UnixNano() == staleTime.UnixNano() },
+		testutil.WaitMedium, testutil.IntervalFast, "expected provisioner LastSeenAt to be:%+v, saw :%+v", staleTime.UTC(), p.LastSeenAt.Time.UTC())
 
-	wg.Wait()
+	// Ensure the provisioner is gone or stale, relative to the artificial next autostart time, before triggering the autobuild.
+	coderdtest.MustWaitForProvisionersUnavailable(t, db, workspace, provisionerDaemonTags, next)
 
+	// Trigger autobuild.
+	tickCh <- next
 	stats := <-statsCh
-
-	// This assertion should FAIL when provisioner is available (not stale), can confirm by commenting out the
-	// UpdateProvisionerLastSeenAt call above.
 	assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available")
 
 	daemon2Closer := coderdtest.NewTaggedProvisionerDaemon(t, api, "name", provisionerDaemonTags)
@@ -1762,12 +1754,18 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) {
 	// Ensure the provisioner is  NOT stale, and see if we get a successful state transition.
 	p, err = coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags)
 	require.NoError(t, err, "Error getting provisioner for workspace")
-	notStaleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second)
+
+	next = sched.Next(workspace.LatestBuild.CreatedAt)
+	notStaleTime := next.Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second)
 	coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, notStaleTime)
+	// Require that the provisioner time has actually been updated to the expected value.
+	p, err = coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags)
+	require.NoError(t, err, "Error getting provisioner for workspace")
+	require.True(t, next.UnixNano() > p.LastSeenAt.Time.UnixNano())
 
 	// Trigger autobuild
 	go func() {
-		tickCh <- sched.Next(workspace.LatestBuild.CreatedAt)
+		tickCh <- next
 		close(tickCh)
 	}()
 	stats = <-statsCh
