Skip to content

Commit 06e12b0

Browse files
mastermanas805claude
mastermanas805
and
claude
authored
ci(worker): gate the integration tests (non-short job + DB services) (#88)
The worker's real-DB integration tests (internal/jobs/*_integration_test.go) skip under `-short` AND when TEST_DATABASE_URL is unset (see propagation_runner_integration_test.go and the testhelpers harness guard). But EVERY worker test workflow ran with `-short` (deploy.yml) or `-race` with no DB service (ci.yml's build-and-test) — and coverage.yml runs `-short` too. Net: the integration tests skipped EVERYWHERE in CI and gated NOTHING. They only ran on a developer box with a DB up. Fix: add a dedicated GATING `integration` job that runs the real-DB integration tests WITHOUT `-short` against a real Postgres+Redis, reusing coverage.yml's proven service block + api-migrations-apply step. Per the two-gate rule (root CLAUDE.md rule 15 / project_api_two_test_gates_ci_and_deploy — hit 2026-05-23 with the NATS service), a worker test-infra change must land in BOTH ci.yml and deploy.yml or the deploy wedges while PR CI stays green. So: - ci.yml: new `integration` job (PR + push gate). - deploy.yml: mirror `integration` job; `deploy` now `needs: integration` so a red integration round-trip blocks build/rollout. The fast `-short` `build-and-test` (ci.yml) and "Run unit tests" (deploy.yml) jobs are unchanged — `make gate` parity (rule 23) holds. Also fixes a pre-existing FK violation in TestPropagation_ForUpdateSkipLockedIntegration: it seeded a pending_propagations row referencing a non-existent team, which fails against a fully api-migrated DB (the FK pending_propagations_team_id_fkey → teams). Without this the new non-short job would red on a pre-existing bug. The test now seeds the parent team first; the FK cascade tidies it. Verified locally against postgres://postgres@localhost:5432/instant_dev_test: - non-short `go test ./internal/... -run Integration -p 1` → all pass (exit 0). - a temporary broken assert in the live-DB SKIP-LOCKED test → reds the run. - `make gate` (the `-short` path) still green; integration tests skip there. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 917fbb0 commit 06e12b0

3 files changed

Lines changed: 228 additions & 0 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,117 @@ jobs:
9898
env:
9999
INSTANT_API_REPO: ${{ github.workspace }}/../api
100100
run: go test ./... -v -race -count=1
101+
102+
# ------------------------------------------------------------------
103+
# GATING integration job (W2-T1).
104+
#
105+
# WHY: the worker's real-DB integration tests
106+
# (internal/jobs/*_integration_test.go) skip under `-short` AND when
107+
# TEST_DATABASE_URL is unset. The fast `build-and-test` job above runs
108+
# `-race` with NO DB service and the deploy.yml test step runs
109+
# `-short` — so in BOTH the integration tests SKIP and gate NOTHING.
110+
# This job runs them WITHOUT `-short` against a real Postgres so the
111+
# trigger→DB round-trip is actually asserted in CI. It mirrors
112+
# coverage.yml's proven Postgres+Redis service block. Per the two-gate
113+
# rule (root CLAUDE.md rule 15 / project_api_two_test_gates_ci_and_deploy),
114+
# the same job is mirrored in deploy.yml — a worker test-infra change
115+
# must land in BOTH or the deploy wedges while PR CI stays green.
116+
integration:
117+
runs-on: ubuntu-latest
118+
timeout-minutes: 15
119+
services:
120+
postgres:
121+
image: postgres:16-alpine
122+
env:
123+
POSTGRES_USER: postgres
124+
POSTGRES_PASSWORD: postgres
125+
POSTGRES_DB: instant_dev_test
126+
ports:
127+
- 5432:5432
128+
options: >-
129+
--health-cmd pg_isready
130+
--health-interval 10s
131+
--health-timeout 5s
132+
--health-retries 5
133+
redis:
134+
image: redis:7-alpine
135+
ports:
136+
- 6379:6379
137+
options: >-
138+
--health-cmd "redis-cli ping"
139+
--health-interval 10s
140+
--health-timeout 5s
141+
--health-retries 5
142+
env:
143+
# Integration tests read TEST_DATABASE_URL and run the real-DB path
144+
# (they SKIP when it's unset). TEST_REDIS_URL is wired for parity
145+
# with coverage.yml and for the redis-touching jobs as they land.
146+
TEST_DATABASE_URL: postgres://postgres:postgres@localhost:5432/instant_dev_test?sslmode=disable
147+
TEST_REDIS_URL: redis://localhost:6379/15
148+
steps:
149+
- uses: actions/checkout@v6
150+
151+
- name: Checkout proto sibling (replace ../proto)
152+
uses: actions/checkout@v6
153+
with:
154+
repository: ${{ vars.PROTO_REPO || format('{0}/proto', github.repository_owner) }}
155+
token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
156+
path: _proto_ci
157+
- run: mv _proto_ci ../proto
158+
159+
- name: Checkout common sibling (replace ../common)
160+
uses: actions/checkout@v6
161+
with:
162+
repository: ${{ vars.COMMON_REPO || format('{0}/common', github.repository_owner) }}
163+
token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
164+
path: _common_ci
165+
- run: mv _common_ci ../common
166+
167+
- name: Checkout api sibling (for migrations + cross-repo tests)
168+
uses: actions/checkout@v6
169+
with:
170+
repository: ${{ vars.API_REPO || format('{0}/api', github.repository_owner) }}
171+
token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
172+
path: _api_ci
173+
fetch-depth: 1
174+
- run: mv _api_ci ../api
175+
176+
- uses: actions/setup-go@v6
177+
with:
178+
go-version: '1.25'
179+
180+
- name: Apply DB migrations to the test database
181+
# Mirrors coverage.yml: api owns the platform_db schema
182+
# (teams / resources / deployments / pending_propagations /
183+
# audit_log) the integration tests round-trip against; apply those
184+
# first, then any worker-local SQL on top (IF NOT EXISTS-guarded).
185+
env:
186+
PGPASSWORD: postgres
187+
run: |
188+
if [ -d ../api/internal/db/migrations ]; then
189+
for f in $(ls ../api/internal/db/migrations/*.sql | sort); do
190+
echo "→ applying api migration $(basename "$f")"
191+
psql -h localhost -U postgres -d instant_dev_test -f "$f" >/dev/null
192+
done
193+
echo "all api migrations applied to instant_dev_test"
194+
else
195+
echo "::error::no api migrations directory found at ../api/internal/db/migrations — integration tests would skip"
196+
exit 1
197+
fi
198+
if [ -d sql ]; then
199+
for f in $(ls sql/*.sql 2>/dev/null | sort); do
200+
echo "→ applying worker migration $(basename "$f")"
201+
psql -h localhost -U postgres -d instant_dev_test -f "$f" >/dev/null || echo "::warning::worker migration $(basename "$f") failed — likely overlap with api schema; continuing"
202+
done
203+
fi
204+
205+
- name: Run real-DB integration tests (NO -short, so they do not skip)
206+
# `-run Integration` scopes the run to the _integration_test.go
207+
# funcs (all carry "Integration" in their name); `-p 1` because
208+
# every package shares the single instant_dev_test DB and default
209+
# parallelism corrupts shared state. Dropping `-short` is what
210+
# makes testhelpers.SetupTestDB's testing.Short() guard pass and
211+
# the real-DB path run instead of skipping.
212+
env:
213+
INSTANT_API_REPO: ${{ github.workspace }}/../api
214+
run: go test ./internal/... -run Integration -count=1 -p 1 -v

‎.github/workflows/deploy.yml‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,109 @@ env:
5454
K8S_CONTAINER: worker
5555

5656
jobs:
57+
# ------------------------------------------------------------------
58+
# GATING integration job (W2-T1) — two-gate rule mirror of ci.yml.
59+
#
60+
# The deploy.yml test step ("Run unit tests") runs `go test ./...
61+
# -short`, under which every worker real-DB integration test SKIPS
62+
# (testhelpers.SetupTestDB / *_integration_test.go guard on
63+
# testing.Short()). So before this job, a broken integration
64+
# round-trip could merge to master and DEPLOY — the deploy gate never
65+
# ran the integration tests. Per project_api_two_test_gates_ci_and_deploy
66+
# (root CLAUDE.md rule 15), a worker test-infra change must land in
67+
# BOTH ci.yml and deploy.yml or the deploy wedges while PR CI stays
68+
# green. `deploy` now `needs: integration`, so a red integration run
69+
# blocks the build/rollout.
70+
integration:
71+
runs-on: ubuntu-latest
72+
timeout-minutes: 15
73+
services:
74+
postgres:
75+
image: postgres:16-alpine
76+
env:
77+
POSTGRES_USER: postgres
78+
POSTGRES_PASSWORD: postgres
79+
POSTGRES_DB: instant_dev_test
80+
ports:
81+
- 5432:5432
82+
options: >-
83+
--health-cmd pg_isready
84+
--health-interval 10s
85+
--health-timeout 5s
86+
--health-retries 5
87+
redis:
88+
image: redis:7-alpine
89+
ports:
90+
- 6379:6379
91+
options: >-
92+
--health-cmd "redis-cli ping"
93+
--health-interval 10s
94+
--health-timeout 5s
95+
--health-retries 5
96+
env:
97+
TEST_DATABASE_URL: postgres://postgres:postgres@localhost:5432/instant_dev_test?sslmode=disable
98+
TEST_REDIS_URL: redis://localhost:6379/15
99+
steps:
100+
- uses: actions/checkout@v6
101+
102+
- name: Checkout proto sibling (replace ../proto)
103+
uses: actions/checkout@v6
104+
with:
105+
repository: ${{ vars.PROTO_REPO || format('{0}/proto', github.repository_owner) }}
106+
token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
107+
path: _proto_ci
108+
- run: mv _proto_ci ../proto
109+
110+
- name: Checkout common sibling (replace ../common)
111+
uses: actions/checkout@v6
112+
with:
113+
repository: ${{ vars.COMMON_REPO || format('{0}/common', github.repository_owner) }}
114+
token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
115+
path: _common_ci
116+
- run: mv _common_ci ../common
117+
118+
- name: Checkout api sibling (for migrations + cross-repo tests)
119+
uses: actions/checkout@v6
120+
with:
121+
repository: ${{ vars.API_REPO || format('{0}/api', github.repository_owner) }}
122+
token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
123+
path: _api_ci
124+
fetch-depth: 1
125+
- run: mv _api_ci ../api
126+
127+
- uses: actions/setup-go@v6
128+
with:
129+
go-version: '1.25'
130+
131+
- name: Apply DB migrations to the test database
132+
env:
133+
PGPASSWORD: postgres
134+
run: |
135+
if [ -d ../api/internal/db/migrations ]; then
136+
for f in $(ls ../api/internal/db/migrations/*.sql | sort); do
137+
echo "→ applying api migration $(basename "$f")"
138+
psql -h localhost -U postgres -d instant_dev_test -f "$f" >/dev/null
139+
done
140+
echo "all api migrations applied to instant_dev_test"
141+
else
142+
echo "::error::no api migrations directory found at ../api/internal/db/migrations — integration tests would skip"
143+
exit 1
144+
fi
145+
if [ -d sql ]; then
146+
for f in $(ls sql/*.sql 2>/dev/null | sort); do
147+
echo "→ applying worker migration $(basename "$f")"
148+
psql -h localhost -U postgres -d instant_dev_test -f "$f" >/dev/null || echo "::warning::worker migration $(basename "$f") failed — likely overlap with api schema; continuing"
149+
done
150+
fi
151+
152+
- name: Run real-DB integration tests (NO -short, so they do not skip)
153+
env:
154+
INSTANT_API_REPO: ${{ github.workspace }}/../api
155+
run: go test ./internal/... -run Integration -count=1 -p 1 -v
156+
57157
deploy:
58158
runs-on: ubuntu-latest
159+
needs: integration
59160
steps:
60161
- name: Checkout worker (this repo) into ./worker
61162
uses: actions/checkout@v6

‎internal/jobs/propagation_runner_integration_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,19 @@ func TestPropagation_ForUpdateSkipLockedIntegration(t *testing.T) {
316316
ctx := context.Background()
317317
teamID := uuid.New()
318318
propID := uuid.New()
319+
// pending_propagations.team_id carries a real FK to teams(id) in the api
320+
// migration set (ON DELETE CASCADE). Against a fully-migrated DB (the
321+
// non-short integration job's environment) the seed must reference a real
322+
// team row or the INSERT fails the FK. Seed the parent team first; the FK
323+
// cascade tidies the propagation row when the team is deleted in cleanup.
324+
if _, err := db.ExecContext(ctx,
325+
`INSERT INTO teams (id) VALUES ($1)`, teamID,
326+
); err != nil {
327+
t.Fatalf("seed team: %v", err)
328+
}
329+
t.Cleanup(func() {
330+
_, _ = db.ExecContext(context.Background(), `DELETE FROM teams WHERE id = $1`, teamID)
331+
})
319332
if _, err := db.ExecContext(ctx, `
320333
INSERT INTO pending_propagations
321334
(id, kind, team_id, target_tier, payload, attempts, next_attempt_at, created_at)

0 commit comments

Comments
 (0)