Skip to content

Commit 088f61c

Browse files
test(coverage): worker internal/circuit + jobs tail → ≥95% (CI-measured) (#59)
Closes the last two per-module coverage gaps in the worker repo, measured under the EXISTING coverage.yml CI environment (postgres/redis/mongo service containers + TEST_* env) — NOT relying on TEST_WORKER_STARTUP_DSN, which is unset in CI so its StartWorkers boot test skips there. internal/circuit 91.8% → 100.0% - NewBreaker: threshold<1 clamp + cooldown<=0 default arms - State(): half_open / within-cooldown / after-cooldown-before-trial arms - Name(): metric-label accessor (These tests mirror the api/internal/circuit copy by the file's own lock-step contract — apply the same additions there in a follow-up.) internal/jobs 94.7% → 95.1% Reachable arms covered with sqlmock + an SDK-disabled New Relic app + pure-value calls (no live infra): - middleware Work: nrApp != nil transaction path + txn.NoticeError arm - event_email_mapping build{BackupFailed,RestoreSucceeded,RestoreFailed}: the row.ResourceType != "" column-wins arm - billing_reconciler emit{Upgrade,Cancel}Audit: fail-open err != nil arm - checkout_reconcile emailAbandonedCheckout: non-ErrNoRows claim error arm - provisioner_reconciler markAbandoned: UPDATE-error arm Otherwise-unreachable json.Marshal degradation arms (writeAudit ×3, insertPropagationAuditRow) covered by passing an unmarshalable meta map (chan value) — no source-level seam required. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 23d0f32 commit 088f61c

2 files changed

Lines changed: 349 additions & 0 deletions

File tree

internal/circuit/circuit_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,88 @@ func TestBreaker_OnOpenCallback(t *testing.T) {
160160
}
161161
}
162162

163+
// TestBreaker_NewBreakerClampsInvalidArgs — threshold < 1 is clamped to 1
164+
// and a non-positive cooldown defaults to 30s. Exercises the two guard
165+
// branches in NewBreaker (otherwise only the happy path is hit).
166+
func TestBreaker_NewBreakerClampsInvalidArgs(t *testing.T) {
167+
// threshold 0 → clamped to 1: a single failure must trip the breaker.
168+
b := NewBreaker("worker_test_clamp_threshold", 0, 10*time.Millisecond)
169+
if !b.Allow() {
170+
t.Fatal("fresh breaker should allow")
171+
}
172+
b.Record(errBoom)
173+
if b.State() != StateOpen {
174+
t.Fatalf("threshold should clamp to 1 (single failure opens), got %s", b.State())
175+
}
176+
177+
// cooldown <= 0 → defaults to 30s. We can't wait 30s, but we can prove
178+
// the breaker is still open well past a tiny sleep (a 0 cooldown would
179+
// have re-admitted immediately).
180+
b2 := NewBreaker("worker_test_clamp_cooldown", 1, 0)
181+
_ = b2.Allow()
182+
b2.Record(errBoom)
183+
time.Sleep(5 * time.Millisecond)
184+
if b2.Allow() {
185+
t.Fatal("cooldown should default to 30s; breaker must still reject after 5ms")
186+
}
187+
if b2.State() != StateOpen {
188+
t.Fatalf("expected open within default cooldown, got %s", b2.State())
189+
}
190+
}
191+
192+
// TestBreaker_StateOpenWithinCooldown — when the breaker is open and the
193+
// cooldown has NOT elapsed, State() takes the `now < openUntilNs` branch
194+
// and reports open. Distinct from the half-open trial path.
195+
func TestBreaker_StateOpenWithinCooldown(t *testing.T) {
196+
b := NewBreaker("worker_test_state_within_cooldown", 1, time.Hour)
197+
_ = b.Allow()
198+
b.Record(errBoom)
199+
// halfOpen is false, openUntil is set, now < openUntil → first return.
200+
if got := b.State(); got != StateOpen {
201+
t.Fatalf("breaker within cooldown should report open, got %s", got)
202+
}
203+
}
204+
205+
// TestBreaker_StateOpenAfterCooldownBeforeTrial — once the cooldown has
206+
// elapsed but no caller has claimed the half-open trial yet, State() falls
207+
// through past the `now < openUntilNs` check and still reports open (the
208+
// final return). This exercises the trailing branch of State().
209+
func TestBreaker_StateOpenAfterCooldownBeforeTrial(t *testing.T) {
210+
b := NewBreaker("worker_test_state_after_cooldown", 1, 10*time.Millisecond)
211+
_ = b.Allow()
212+
b.Record(errBoom)
213+
time.Sleep(15 * time.Millisecond)
214+
// Cooldown elapsed, but we have NOT called Allow() — so halfOpen is
215+
// still false and openUntil is still in the past.
216+
if got := b.State(); got != StateOpen {
217+
t.Fatalf("breaker after cooldown but before trial should report open, got %s", got)
218+
}
219+
}
220+
221+
// TestBreaker_StateHalfOpenReported — once a caller claims the half-open
222+
// trial slot (Allow() after cooldown), State() takes the leading
223+
// `halfOpen.Load()` branch and reports half_open.
224+
func TestBreaker_StateHalfOpenReported(t *testing.T) {
225+
b := NewBreaker("worker_test_state_half_open", 1, 10*time.Millisecond)
226+
_ = b.Allow()
227+
b.Record(errBoom)
228+
time.Sleep(15 * time.Millisecond)
229+
if !b.Allow() {
230+
t.Fatal("first Allow() after cooldown should claim the half-open trial")
231+
}
232+
if got := b.State(); got != StateHalfOpen {
233+
t.Fatalf("breaker mid-trial should report half_open, got %s", got)
234+
}
235+
}
236+
237+
// TestBreaker_Name — the metric-label accessor returns the configured name.
238+
func TestBreaker_Name(t *testing.T) {
239+
b := NewBreaker("worker_test_name_accessor", 1, time.Second)
240+
if got := b.Name(); got != "worker_test_name_accessor" {
241+
t.Fatalf("Name() = %q, want %q", got, "worker_test_name_accessor")
242+
}
243+
}
244+
163245
// TestBreaker_StateStringValues — NR runbook references these exact
164246
// strings.
165247
func TestBreaker_StateStringValues(t *testing.T) {
Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
package jobs
2+
3+
// coverage_tail_95_test.go — closes the last reachable per-function gaps in
4+
// the jobs package so the CI-measured package total clears the 95% floor.
5+
//
6+
// Every test here runs under the EXISTING coverage.yml CI environment
7+
// (postgres/redis/mongo service containers + TEST_* env). NONE of them
8+
// depend on TEST_WORKER_STARTUP_DSN — that env var is NOT set in CI, so the
9+
// StartWorkers boot test it gates SKIPS there. We add coverage via
10+
// sqlmock + an SDK-disabled New Relic application + pure-value calls that
11+
// need no live infra at all.
12+
//
13+
// Targets (each was < 95% in the CI-measured profile):
14+
// * middleware.go Work — the w.nrApp != nil transaction path +
15+
// the txn.NoticeError(err) error arm.
16+
// * event_email_mapping buildBackupFailed / buildRestoreSucceeded /
17+
// buildRestoreFailed — the `row.ResourceType != ""`
18+
// column-wins branch (the metadata-fallback else
19+
// branch is already covered).
20+
// * billing_reconciler emitUpgradeAudit / emitCancelAudit — the
21+
// fail-open `err != nil` arm.
22+
// * checkout_reconcile emailAbandonedCheckout — the claim-row
23+
// non-ErrNoRows DB-error arm.
24+
25+
import (
26+
"context"
27+
"errors"
28+
"testing"
29+
30+
sqlmock "github.com/DATA-DOG/go-sqlmock"
31+
"github.com/google/uuid"
32+
"github.com/newrelic/go-agent/v3/newrelic"
33+
34+
"instant.dev/common/logctx"
35+
)
36+
37+
// newDisabledNRApp builds a real, non-nil *newrelic.Application whose
38+
// transactions are no-ops and which performs NO network I/O. ConfigEnabled(false)
39+
// is the SDK's documented hermetic mode — StartTransaction returns a live
40+
// (but inert) *Transaction, exercising the wrapper's nrApp != nil path
41+
// without a daemon, a license key, or any harvest cycle.
42+
func newDisabledNRApp(t *testing.T) *newrelic.Application {
43+
t.Helper()
44+
app, err := newrelic.NewApplication(
45+
newrelic.ConfigAppName("instant-worker-test"),
46+
newrelic.ConfigEnabled(false),
47+
)
48+
if err != nil {
49+
t.Fatalf("newrelic.NewApplication(disabled): %v", err)
50+
}
51+
return app
52+
}
53+
54+
// TestWithObservability_NRPresent_Success drives the w.nrApp != nil arm of
55+
// Work on the success path: StartTransaction + NewContext + defer End all
56+
// execute, then the inner worker returns nil.
57+
func TestWithObservability_NRPresent_Success(t *testing.T) {
58+
fake := &fakeWorker{}
59+
wrapped := WithObservability[fakeArgs](fake, newDisabledNRApp(t))
60+
61+
if err := wrapped.Work(context.Background(), newJob(42)); err != nil {
62+
t.Fatalf("Work returned error on success path: %v", err)
63+
}
64+
// The wrapper must still have stamped the ctx ids on the way through the
65+
// NR-present branch — same contract as the nil-app path.
66+
if got := logctx.TIDFromContext(fake.gotCtx); got == "" {
67+
t.Error("tid not stamped on ctx in NR-present path")
68+
}
69+
if got := logctx.TraceIDFromContext(fake.gotCtx); got == "" {
70+
t.Error("trace_id not stamped on ctx in NR-present path")
71+
}
72+
}
73+
74+
// TestWithObservability_NRPresent_Error drives the txn.NoticeError(err) arm:
75+
// nrApp != nil AND the inner worker fails, so both the transaction-open
76+
// branch and the error-notice branch execute.
77+
func TestWithObservability_NRPresent_Error(t *testing.T) {
78+
wantErr := errors.New("inner work blew up")
79+
fake := &fakeWorker{returns: wantErr}
80+
wrapped := WithObservability[fakeArgs](fake, newDisabledNRApp(t))
81+
82+
err := wrapped.Work(context.Background(), newJob(7))
83+
if !errors.Is(err, wantErr) {
84+
t.Fatalf("Work should bubble the inner error unchanged, got %v", err)
85+
}
86+
}
87+
88+
// TestEventEmail_Builders_ResourceTypeFromColumn covers the
89+
// `if row.ResourceType != ""` arm of the three backup/restore builders —
90+
// when the audit row carries a ResourceType column, it wins over the
91+
// metadata fallback. The else (metadata) arm is covered elsewhere.
92+
func TestEventEmail_Builders_ResourceTypeFromColumn(t *testing.T) {
93+
cases := []struct {
94+
name string
95+
builder func(auditRow) (map[string]string, bool)
96+
}{
97+
{"buildBackupFailed", buildBackupFailed},
98+
{"buildRestoreSucceeded", buildRestoreSucceeded},
99+
{"buildRestoreFailed", buildRestoreFailed},
100+
}
101+
for _, c := range cases {
102+
t.Run(c.name, func(t *testing.T) {
103+
row := auditRow{
104+
ID: "a-rt-1",
105+
TeamID: "t-rt-1",
106+
OwnerEmail: "owner@example.com",
107+
ResourceType: "postgres", // non-empty → column-wins branch
108+
}
109+
params, ok := c.builder(row)
110+
if !ok {
111+
t.Fatalf("%s returned ok=false with a valid owner email", c.name)
112+
}
113+
if params["resource_type"] != "postgres" {
114+
t.Errorf("%s: resource_type = %q; want the column value %q",
115+
c.name, params["resource_type"], "postgres")
116+
}
117+
})
118+
}
119+
}
120+
121+
// TestBillingReconciler_EmitUpgradeAudit_FailOpen drives the fail-open arm of
122+
// emitUpgradeAudit: the audit INSERT errors, so RecordFailOpen runs and the
123+
// method returns without panicking (tier change already committed upstream).
124+
func TestBillingReconciler_EmitUpgradeAudit_FailOpen(t *testing.T) {
125+
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp))
126+
if err != nil {
127+
t.Fatalf("sqlmock.New: %v", err)
128+
}
129+
defer db.Close()
130+
mock.ExpectExec(`INSERT INTO audit_log`).WillReturnError(errors.New("audit DB brownout"))
131+
132+
w := &BillingReconcilerWorker{db: db}
133+
// Must not panic; the fail-open path swallows the error.
134+
w.emitUpgradeAudit(context.Background(), uuid.New(), "hobby", "pro", "sub_x")
135+
136+
if err := mock.ExpectationsWereMet(); err != nil {
137+
t.Errorf("unmet sqlmock expectations: %v", err)
138+
}
139+
}
140+
141+
// TestBillingReconciler_EmitCancelAudit_FailOpen — same fail-open arm for the
142+
// cancel audit row.
143+
func TestBillingReconciler_EmitCancelAudit_FailOpen(t *testing.T) {
144+
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp))
145+
if err != nil {
146+
t.Fatalf("sqlmock.New: %v", err)
147+
}
148+
defer db.Close()
149+
mock.ExpectExec(`INSERT INTO audit_log`).WillReturnError(errors.New("audit DB brownout"))
150+
151+
w := &BillingReconcilerWorker{db: db}
152+
w.emitCancelAudit(context.Background(), uuid.New(), "pro", "hobby", "sub_x")
153+
154+
if err := mock.ExpectationsWereMet(); err != nil {
155+
t.Errorf("unmet sqlmock expectations: %v", err)
156+
}
157+
}
158+
159+
// TestCheckoutReconcile_EmailAbandonedCheckout_ClaimError drives the
160+
// non-ErrNoRows error arm of emailAbandonedCheckout's claim SELECT: a generic
161+
// DB error (not sql.ErrNoRows) must propagate as a wrapped Work error so the
162+
// tx rolls back and the sweep records the failure.
163+
func TestCheckoutReconcile_EmailAbandonedCheckout_ClaimError(t *testing.T) {
164+
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp))
165+
if err != nil {
166+
t.Fatalf("sqlmock.New: %v", err)
167+
}
168+
defer db.Close()
169+
170+
mock.ExpectBegin()
171+
// The claim SELECT returns a generic error (NOT sql.ErrNoRows) → the
172+
// `if err != nil` arm wraps and returns it.
173+
mock.ExpectQuery(`SELECT subscription_id\s+FROM pending_checkouts`).
174+
WithArgs("sub_claim_err").
175+
WillReturnError(errors.New("lock wait timeout"))
176+
mock.ExpectRollback()
177+
178+
w := &CheckoutReconcileWorker{db: db}
179+
gotErr := w.emailAbandonedCheckout(context.Background(), checkoutRow{
180+
subscriptionID: "sub_claim_err",
181+
teamID: uuid.New().String(),
182+
customerEmail: "buyer@example.com",
183+
planTier: "pro",
184+
})
185+
if gotErr == nil {
186+
t.Fatal("expected a wrapped claim-row error, got nil")
187+
}
188+
}
189+
190+
// unmarshalableMeta returns a metadata map that json.Marshal genuinely
191+
// cannot encode (a channel value has no JSON representation). This drives
192+
// the audit-row marshal-error degradation arms that are otherwise
193+
// unreachable with primitive-only maps — without any source-level seam.
194+
func unmarshalableMeta() map[string]any {
195+
return map[string]any{"bad": make(chan int)}
196+
}
197+
198+
// TestCustomerRestoreRunner_WriteAudit_MarshalError drives the
199+
// audit_marshal_failed degradation arm: an unmarshalable meta map makes
200+
// json.Marshal fail, so writeAudit logs + returns BEFORE touching the DB
201+
// (db is nil here, proving the early return).
202+
func TestCustomerRestoreRunner_WriteAudit_MarshalError(t *testing.T) {
203+
w := &CustomerRestoreRunnerWorker{db: nil}
204+
// Must not panic and must not dereference the nil db — the marshal
205+
// failure short-circuits ahead of ExecContext.
206+
w.writeAudit(context.Background(), uuid.New(), uuid.New().String(),
207+
"postgres", "restore.failed", "summary", unmarshalableMeta())
208+
}
209+
210+
// TestCustomerBackupRunner_WriteAudit_MarshalError — same degradation arm
211+
// on the backup runner's writeAudit.
212+
func TestCustomerBackupRunner_WriteAudit_MarshalError(t *testing.T) {
213+
w := &CustomerBackupRunnerWorker{db: nil}
214+
w.writeAudit(context.Background(), uuid.New(), uuid.New().String(),
215+
"postgres", "backup.failed", "summary", unmarshalableMeta())
216+
}
217+
218+
// TestPlatformDBBackup_WriteAudit_MarshalError — the platform-DB backup
219+
// writeAudit checks w.db == nil first, so we pass a sqlmock DB (with NO
220+
// expectations: the marshal failure returns before any ExecContext).
221+
func TestPlatformDBBackup_WriteAudit_MarshalError(t *testing.T) {
222+
db, mock, err := sqlmock.New()
223+
if err != nil {
224+
t.Fatalf("sqlmock.New: %v", err)
225+
}
226+
defer db.Close()
227+
w := &PlatformDBBackupWorker{db: db}
228+
w.writeAudit(context.Background(), "backup.failed", "summary", unmarshalableMeta())
229+
// No DB call expected — the marshal error returns first.
230+
if err := mock.ExpectationsWereMet(); err != nil {
231+
t.Errorf("unexpected DB call on marshal-error path: %v", err)
232+
}
233+
}
234+
235+
// TestPropagationRunner_InsertAuditRow_MarshalError drives the
236+
// audit_meta_marshal_failed arm of insertPropagationAuditRow: an
237+
// unmarshalable meta map short-circuits before the INSERT (db nil proves it).
238+
func TestPropagationRunner_InsertAuditRow_MarshalError(t *testing.T) {
239+
w := &PropagationRunnerWorker{db: nil}
240+
w.insertPropagationAuditRow(context.Background(),
241+
propagationRow{id: uuid.New(), teamID: uuid.New(), kind: "regrade"},
242+
"propagation.failed", "summary", unmarshalableMeta())
243+
}
244+
245+
// TestProvisionerReconciler_MarkAbandoned_UpdateError drives the reachable
246+
// `UPDATE resources ... err != nil` arm of markAbandoned: a DB error on the
247+
// status flip must propagate (the audit INSERT is never reached).
248+
func TestProvisionerReconciler_MarkAbandoned_UpdateError(t *testing.T) {
249+
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp))
250+
if err != nil {
251+
t.Fatalf("sqlmock.New: %v", err)
252+
}
253+
defer db.Close()
254+
mock.ExpectExec(`UPDATE resources`).
255+
WillReturnError(errors.New("update blew up"))
256+
257+
w := &ProvisionerReconcilerWorker{db: db}
258+
gotErr := w.markAbandoned(context.Background(),
259+
reconcilerCandidate{id: uuid.New(), token: uuid.New(), resourceType: "postgres"},
260+
errors.New("probe failed"))
261+
if gotErr == nil {
262+
t.Fatal("expected the UPDATE error to propagate, got nil")
263+
}
264+
if err := mock.ExpectationsWereMet(); err != nil {
265+
t.Errorf("unmet sqlmock expectations: %v", err)
266+
}
267+
}

0 commit comments

Comments
 (0)