diff --git a/TODO.md b/TODO.md index 1f8ab6a..ae65dff 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,10 @@ main cannot regress. # Completed Steps +- 2026-09-22: Fixed the flaky `t.TempDir` cleanup race in + `internal/service/webhook` by tracking the async deployment goroutine + in a `sync.WaitGroup` and exposing `WaitForDeployments`; tests now + synchronize on completion instead of sleeping (#198). - 2026-09-22: Linting now runs only in Docker. Added `Dockerfile.lint` (pinned golangci-lint v2.12.2, cache-busted via a `GATE_RUN` build arg so the linter always executes), reduced `script/lint` to building it, diff --git a/internal/service/webhook/webhook.go b/internal/service/webhook/webhook.go index 69c1f6c..02a51ee 100644 --- a/internal/service/webhook/webhook.go +++ b/internal/service/webhook/webhook.go @@ -6,6 +6,7 @@ import ( "database/sql" "fmt" "log/slog" + "sync" "go.uber.org/fx" @@ -31,6 +32,10 @@ type Service struct { db *database.Database deploy *deploy.Service params *ServiceParams + + // deployments tracks the deployment goroutines started by + // triggerDeployment so callers can wait for them to finish. + deployments sync.WaitGroup } // New creates a new webhook Service. @@ -108,6 +113,14 @@ func (svc *Service) HandleWebhook( return nil } +// WaitForDeployments blocks until every deployment goroutine started by +// HandleWebhook has finished, including all writes under the data +// directory. It exists so callers and tests can synchronize on async +// deployment completion instead of polling or sleeping. +func (svc *Service) WaitForDeployments() { + svc.deployments.Wait() +} + func (svc *Service) triggerDeployment( ctx context.Context, app *models.App, @@ -117,7 +130,7 @@ func (svc *Service) triggerDeployment( eventID := event.ID appName := app.Name - go func() { + svc.deployments.Go(func() { // Use context.WithoutCancel to ensure deployment completes // even if the HTTP request context is cancelled. deployCtx := context.WithoutCancel(ctx) @@ -130,5 +143,5 @@ func (svc *Service) triggerDeployment( // Mark event as processed event.Processed = true _ = event.Save(deployCtx) - }() + }) } diff --git a/internal/service/webhook/webhook_test.go b/internal/service/webhook/webhook_test.go index bda7a98..619b271 100644 --- a/internal/service/webhook/webhook_test.go +++ b/internal/service/webhook/webhook_test.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -828,8 +827,9 @@ func TestExtractBranch(testingT *testing.T) { ) require.NoError(t, err) - // Allow async deployment goroutine to complete before test cleanup - time.Sleep(100 * time.Millisecond) + // Wait for the async deployment goroutine to finish so its + // writes under the temp dir complete before test cleanup. + svc.WaitForDeployments() events, err := app.GetWebhookEvents(context.Background(), 10) require.NoError(t, err) @@ -867,8 +867,9 @@ func TestHandleWebhookMatchingBranch(t *testing.T) { ) require.NoError(t, err) - // Allow async deployment goroutine to complete before test cleanup - time.Sleep(100 * time.Millisecond) + // Wait for the async deployment goroutine to finish so its writes + // under the temp dir complete before test cleanup. + svc.WaitForDeployments() events, err := app.GetWebhookEvents(context.Background(), 10) require.NoError(t, err) @@ -962,8 +963,9 @@ func assertHandleWebhookDeploys( err := svc.HandleWebhook(context.Background(), app, source, pushEventType, payload) require.NoError(t, err) - // Allow async deployment goroutine to complete before test cleanup - time.Sleep(100 * time.Millisecond) + // Wait for the async deployment goroutine to finish so its writes + // under the temp dir complete before test cleanup. + svc.WaitForDeployments() events, err := app.GetWebhookEvents(context.Background(), 10) require.NoError(t, err)