fix(test): stop the live smoke suite inheriting the loopback server list - #14583
Conversation
playwright.live.config.ts built itself with defineConfig(baseConfig, {...}),
expecting its webServer list to replace the base one. Multi-argument
defineConfig concatenates array options instead, so the live run started five
servers rather than two: the loopback OpenAI fixture, the base backend on 7860
pointed at that fixture, the base frontend on 3000, then the live backend on
7861 and a second frontend on 3000. The duplicate port 3000 with
reuseExistingServer:false failed the job before a single test ran.
The port clash was masking the real defect. Without it the suite would have
driven the base frontend, whose proxy targets the 7860 backend running against
the loopback fixture, so every "live provider" assertion would have passed
without reaching a real provider -- exactly the leak the config's comment says
it prevents.
Spreading baseConfig overrides webServer instead of appending to it. Verified
by bundling the real config: 5 webServer entries before (two on port 3000),
2 after (7861 backend, 3000 frontend), with testDir and testIgnore unchanged.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe Playwright live configuration now spreads ChangesPlaywright live configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change makes the live smoke suite start only its real-provider backend and frontend instead of inheriting loopback fixtures and a duplicate port-3000 server. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14583 +/- ##
==================================================
- Coverage 64.82% 59.08% -5.75%
==================================================
Files 2454 2417 -37
Lines 250987 241646 -9341
Branches 34977 19885 -15092
==================================================
- Hits 162709 142782 -19927
- Misses 86214 96800 +10586
Partials 2064 2064
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The
Live provider smoke testsjob failed on its first ever run (nightly 31863695147) before executing a single test:Cause
playwright.live.config.tsbuilds itself withdefineConfig(baseConfig, {...}), expecting itswebServerlist to replace the base one. Multi-argumentdefineConfigconcatenates array options instead. The live run therefore starts five servers:OPENAI_BASE_URL=127.0.0.1:8787/v1npm start, proxy → 7860npm start, proxy → 7861reuseExistingServer: falseturns the duplicate port into a hard failure. The job log matches exactly: two complete alembic migration runs inside oneplaywright testinvocation, then the port error.Why this matters more than a red X
The clash was masking the real defect. Without it, the tests would have driven the base frontend on 3000 — whose proxy targets the 7860 backend wired to the loopback fixture — and every "live provider" assertion would have passed without ever reaching a real provider. That is precisely the leak the config's own comment claims to prevent:
The crash was preventing a false green.
Fix
Spread
baseConfiginstead of passing it asdefineConfig's first argument, sowebServeris overridden rather than appended to. A comment records why, since reverting to the two-argument form silently reintroduces the bug.Verification
Bundled the real config with esbuild and inspected the resolved
webServerlist:testDir: ./tests/liveandtestIgnore: []are unchanged by the switch, as areprojectsanduse.Notes
playwright.live.config.tsarrived in d65b98f (#14540); no earlier nightly ran a smoke job at all, so this is the config's first execution. The job iscontinue-on-error: true, so it is not blocking the nightly.🤖 Generated with Claude Code
Summary by CodeRabbit