test: replace artificial with built-in fetch for integration tests#269
Merged
Conversation
artificial was the source of 7 of 11 npm install deprecation warnings. It transitively pulled in @hapi/joi@16, @hapi/shot@4, and the rest of the deprecated v8/v16 @Hapi line (hoek, address, formula, pinpoint, topo). The package itself has not been updated in years. Drives the Express app over real HTTP via app.listen(0) and Node's built-in fetch instead. The async inject shim awaits listen, fetch, optional callback, and full httpServer.close before resolving, so node:test sees a complete unit of work and exits cleanly without relying on --test-force-exit or a global undici dispatcher hook. Tightens two over-broad header assertions to check what celebrate actually contributes (a Joi-applied default) instead of the full header bag the transport happens to set, and adds res.send() to four route handlers that previously relied on @hapi/shot's fake response semantics. No production deps change; no public API change. Verified on Node 20.19 with both Express 4.22 and Express 5.2: 62/62 tests pass, 100% coverage holds, no deprecated @hapi/* in npm ls --all. Closes #266 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #269 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 263 263
=========================================
Hits 263 263 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Closes #266.
artificialis the source of 7 of the 11npm warn deprecatednotices on a freshnpm install. It pulls in the deprecated@hapi/joi@16and@hapi/shot@4lines, which transitively bring@hapi/hoek@8,@hapi/address@2,@hapi/formula@1,@hapi/pinpoint@1, and@hapi/topo@3.artificialitself has not been updated in years, and we use it in exactly one place:test/integration.test.js, where it attaches an in-processinject(...)method on the Express app via@hapi/shot's simulatedRequest/Response.What's different now
test/integration.test.jsdrives the Express app over real HTTP viaapp.listen(0)and Node's built-infetch. A smallinjectshim inside theServer()factory:fetchper call with a defaultuser-agent: 'shot'(kept so existing assertions stay byte-for-byte) andConnection: closeso the server tears down promptly,httpServer.closebefore resolving.Because the shim is
asyncand everyserver.inject(...)call site nowawaits it,node:testsees a complete unit of work and exits cleanly. No reliance on--test-force-exit, no global undici dispatcher hook, and no new dependency.Two side-effects of going from Shot's fake response to real HTTP:
update req values > req.headersandmultiple-runs > continues to set default valuestests previously asserted on the entirereq.headersbag. With real HTTP that bag includes whatever the transport happens to inject (e.g.accept-language,sec-fetch-mode). Those tests' actual intent is "the Joi default'secret-header': '@@@@@@'got applied toreq.headers," so the assertions are narrowed to that single contract. We're testing celebrate's behavior, not Express + the transport's.update req valuesblock previously got away without sending a response because Shot faked the response cycle. Realfetchhangs forever waiting for one, so each of those handlers now callsres.send()afterteam.attend(req).Tradeoffs
Real HTTP is slightly slower than in-process injection, but the full integration suite still finishes in ~330ms on Node 20. In exchange, the tests now exercise the actual production code path: Node's HTTP parser,
Express.json()reading bytes off a real stream,cookie-parserreading a realCookie:header, and Express 4 vs Express 5 routing/body parsing differences. That's exactly the regime where future regressions would otherwise ship undetected.Teamworkis preserved. The optionalinject(options, callback)shape is preserved. No public API changes; no production dependency changes; no CI matrix changes; nolib/changes.QA Spec
npm installemits zero@hapi/*deprecation warnings.npm ls --allshows no@hapi/joi@16,@hapi/shot,@hapi/hoek@8,@hapi/address@2,@hapi/formula@1,@hapi/pinpoint@1, or@hapi/topo@3.npx eslint .is clean.npm run test:cipasses locally withc8 --100green and the process exits cleanly (no hang).[20.x, 22.x, 24.x, 25.x] x [latest-4, latest]).test/celebrate.test.js.package-lock.jsonaccidentally committed.Made with Cursor