Skip to content

test(mock): make the delayed-abort test discriminate instead of asserting true - #5763

Open
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:test/hardening-verify
Open

test(mock): make the delayed-abort test discriminate instead of asserting true#5763
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:test/hardening-verify

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Problem

The first test in test/mock-delayed-abort.js ended with:

p.ok(true, 'No uncaught errors after delayed response')

That assertion is a tautology — it stays green no matter what the code under test does. The bare MockAgent.request path throws no uncaught error when a delayed reply is delivered after an abort, so the test passed even against a fully-broken guard. It documented an intent it never actually enforced: that a mocked reply delivered after the request was aborted must not still be pushed to the handler.

What this hardens

The post-abort guards live in lib/mock/mock-utils.js (the timer-clear on abort plus the two if (aborted) return guards in handleReply). To prove the test now discriminates, I reverted those three guards and re-ran:

variant test 1 (this one)
real code passes
guards reverted fails

Previously, test 1 passed in both variants.

The change

The delayed-abort test now wraps the dispatched handler in a DecoratorHandler subclass that records any onResponseStart / onResponseData / onResponseEnd delivered after onResponseError, and asserts none occur:

p.strictEqual(deliveredAfterError, false)

This covers the full response side of the contract for the bare-agent path — the sibling DecoratorHandler test in the same file asserts only onResponseStart. Test-only change; no source touched.

Verified with the repo runner: borp -p "test/mock-*.js"339 pass / 0 fail; npm run lint → clean.

…ting true

The first case in test/mock-delayed-abort.js ended with p.ok(true, 'No
uncaught errors after delayed response'), a tautology: the bare MockAgent
path throws no uncaught error when a delayed reply is delivered on top of
an abort, so the assertion stayed green even against code that dropped the
post-abort guards. Verified by reverting the aborted-guards in
lib/mock/mock-utils.js — only the composed-interceptor and DecoratorHandler
cases failed; this one still passed.

Wrap the dispatch handler in a DecoratorHandler that records any
onResponseStart/onResponseData/onResponseEnd delivered after onResponseError
and assert none occur. It now fails against the reverted guards and passes
against real code, covering the full response side of the contract for the
bare-agent path (the sibling test asserts only onResponseStart).
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.50%. Comparing base (17117f6) to head (c18375c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5763   +/-   ##
=======================================
  Coverage   93.50%   93.50%           
=======================================
  Files         110      110           
  Lines       39072    39072           
=======================================
+ Hits        36534    36535    +1     
+ Misses       2538     2537    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants