Fix flaky t.TempDir cleanup race in webhook tests #201
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
}()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user