fix: enable parallel execution for federation integration tests#1385
fix: enable parallel execution for federation integration tests#1385
Conversation
Move mutable global state into per-Resolver instances so each FederationSetup gets isolated data. This eliminates the global mutex that serialized all tests and fixes the data race in subscription resolvers by copying products before mutation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughMove package-level test and fixture globals into resolver instances and factory functions, remove synchronized setup teardown, add parallel test execution ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@execution/federationtesting/products/graph/schema.resolvers.go`:
- Line 24: The TopProducts resolver currently returns
r.products[:len(r.products)-1] which will panic on empty slices and ignores the
first parameter; update the TopProducts function to first guard for empty
r.products (return nil, nil or empty slice) and then compute an upper bound as
min(len(r.products), first) (also handle non-positive first by returning empty)
before slicing (e.g., return r.products[:bound], nil). Ensure you reference and
use r.products and the first parameter in TopProducts to safely clamp the slice.
- Around line 35-48: The subscription goroutine mutates shared resolver state
r.currentPrice without synchronization; add a sync.Mutex field (e.g., priceMu
sync.Mutex) to the Resolver type and use priceMu.Lock()/priceMu.Unlock() around
every read/write of r.currentPrice in the UpdatedPrice handler (the section that
sets p.Price = r.currentPrice and increments r.currentPrice), and ensure the
sync package is imported so concurrent subscriptions cannot race on
currentPrice.
🧹 Nitpick comments (1)
execution/federationtesting/products/graph/handler.go (1)
19-19: GlobalwebsocketConnectionscounter may produce misleading values in parallel tests.The
websocketConnectionsatomic counter remains a package-level variable. While atomic operations prevent data races, parallel tests sharing this counter will see aggregated values from all concurrent test instances. Line 34'sStore(0)reset could also clear counts from other parallel tests mid-execution.If
WebsocketConnectionsHandleris only used for debugging and not asserted in tests, this is acceptable. Otherwise, consider moving the counter into the resolver or handler struct for isolation.Also applies to: 34-34
…rice Address PR review feedback: - Guard TopProducts against empty products slice and respect the first parameter - Add sync.Mutex to protect r.currentPrice from concurrent subscription races Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@execution/federationtesting/products/graph/schema.resolvers.go`:
- Around line 42-57: The case branch can panic when r.products is empty or has
one element; add a guard using n := len(r.products) and if n == 0 { continue }
before accessing r.products, and when picking a random product use idx :=
rand.Intn(n) (rand.Intn requires >0) so it works for n==1 (returns 0); replace
direct uses of r.products[len(r.products)-1] and rand.Intn(len(r.products)-1)
with r.products[n-1] and r.products[idx] respectively, preserving the Price
mutation logic (use p := *product, set p.Price, send via updatedPrice) and keep
synchronization around r.currentPrice with r.priceMu as before.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [1.8.0](execution/v1.7.0...execution/v1.8.0) (2026-02-16) ### Features * add flag for relaxed nullability checks on same shape ([#1378](#1378)) ([6be2e74](6be2e74)) ### Bug Fixes * enable parallel execution for federation integration tests ([#1385](#1385)) ([09d9348](09d9348)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a flag for relaxed nullability checks * **Bug Fixes** * Enabled parallel execution for federation integration tests <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Move mutable global state into per-Resolver instances so each FederationSetup gets isolated data. This eliminates the global mutex that serialized all tests and fixes the data race in subscription resolvers by copying products before mutation.
Summary by CodeRabbit
Tests
Refactor
Checklist