fix(toBeRequestedWith): restore postData/response body matching - #2184
fix(toBeRequestedWith): restore postData/response body matching#2184mccmrunal wants to merge 2 commits into
Conversation
Greptile SummaryThe PR restores request and response payload matching in
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported primitive asymmetric matcher issue is addressed by recognizing constructor samples and parsing serialized JSON primitives before comparison.
|
| Filename | Overview |
|---|---|
| src/matchers/mock/toBeRequestedWith.ts | Restores body matching, parses JSON-compatible payloads for asymmetric comparisons, and handles delayed payload collection without leaving the previously reported primitive-matcher defect. |
| test/matchers/mock/toBeRequestedWith.test.ts | Restores payload scenarios and adds focused coverage for delayed bodies, Buffers, JSON primitives, and constructor-based asymmetric matchers. |
| types/expect-webdriverio.d.ts | Expands request and response expectations to include JSON primitive values accepted by the runtime. |
| package.json | Raises the WebdriverIO peer minimum to the release required for mock payload collection. |
| docs/API.md | Documents asynchronous payload collection and the retry limitations of negated assertions. |
Reviews (4): Last reviewed commit: "fix(toBeRequestedWith): require webdrive..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const actualSample = isAsymmetricMatcher(expected) | ||
| ? getAsymmetricMatcherValue(expected) | ||
| : expected |
There was a problem hiding this comment.
When a JSON primitive body such as "42" is checked with expect.any(Number), the extracted matcher value is not classified as JSON-like, so the raw string is passed to the type matcher and a valid body assertion fails. Boolean type matchers encounter the same mismatch.
Knowledge Base Used: Elements-Array and Mock Matchers
a1c4ef2 to
7baa5da
Compare
| const actualSample = isAsymmetricMatcher(expected) | ||
| ? getAsymmetricMatcherValue(expected) | ||
| : expected | ||
|
|
||
| return ( | ||
| actualSample === null || | ||
| typeof actualSample === 'boolean' || | ||
| typeof actualSample === 'number' || | ||
| Array.isArray(actualSample) || | ||
| (typeof actualSample === 'object' && | ||
| actualSample !== null && | ||
| actualSample instanceof RegExp === false) |
There was a problem hiding this comment.
When a JSON primitive body such as "42" or "true" is asserted with expect.any(Number) or expect.any(Boolean), getAsymmetricMatcherValue returns the constructor function, which is not classified as JSON-like. The body therefore remains a serialized string and the type matcher rejects an otherwise valid payload.
Knowledge Base Used: Elements-Array and Mock Matchers
7baa5da to
ee374a0
Compare
|
@dprevost-LMI Pleae review this |
| /** | ||
| * `postData`/`body` are populated asynchronously (via a `network.getData` round-trip in | ||
| * WebDriverInterception), so they may not be attached to a call yet at the very first check. | ||
| * `.not` assertions normally check once, immediately (`wait: 0`), since a mock's call log only | ||
| * grows and can't "become unmatched" - but that immediate check would race the payload | ||
| * collection and could report a false pass. So when `postData`/`response` are part of the | ||
| * expectation, give `.not` the same "wait for a match to appear" retry window a positive | ||
| * assertion gets, instead of deciding on the first, possibly incomplete, snapshot of the call - | ||
| * but ONLY while there's an actual pending candidate (a call that already matches everything | ||
| * else and is just waiting on its body/postData to attach). If no call matches the non-payload | ||
| * criteria at all, the outcome can't change by waiting, so the predicate below sets `abort` and | ||
| * `waitUntil` resolves immediately - keeping the fast, single-check behavior for the common case | ||
| * (wrong URL, no calls made, etc.) while still closing the race for the one case that needs it. | ||
| * `pass` below always means "was a matching call found" either way - `.not` inversion is | ||
| * handled downstream by the test framework, not by this function - so no extra inversion here, | ||
| * only the retry direction passed into `waitUntil` changes. | ||
| */ | ||
| const hasPayloadExpectation = expectedValue.postData !== undefined || expectedValue.response !== undefined | ||
| const waitForPayloadOnNot = isNot && hasPayloadExpectation | ||
|
|
||
| // shared across every `waitUntil` iteration and the later message-building step, so a given | ||
| // postData/body string is JSON.parsed at most once per assertion instead of once per read | ||
| const parseCache: Map<string, ParsedJson> = new Map() | ||
|
|
||
| /** | ||
| * a call matched everything except its payload, and that payload never arrived. Kept from the | ||
| * final iteration so the failure message can explain *why* the body looks empty rather than | ||
| * just diffing against `undefined` - see `payloadCollectionHint()`. | ||
| */ | ||
| let payloadNeverCollected = false | ||
|
|
There was a problem hiding this comment.
Should we leverage waitForResponse from wdio here ?
|
FYI: We could add e2e in the playgrounds too |
2 participants
Closes #2183
The postData/response body matching logic in expect-webdriverio’s
toBeRequestedWith()mock matcher had been silently commented out since the WebdriverIO v9 BiDi migration in August 2024. This meant that assertions checking request or response bodies would pass regardless of what was actually sent, and the bug went unnoticed for nearly two years.After cloning the repo and doing a broader review, I came across this issue along with several other real defects. I traced the problem back to the v9 interception internals and confirmed that the body data itself had not been removed. It had simply been left disconnected during the migration.
I restored the matching logic, re-enabled the disabled tests, and fixed a related timing race affecting
.notassertions, where the body could be checked before it had finished loading asynchronously.A follow-up review exposed a few more issues introduced by the initial fix: incorrect Buffer handling, ambiguity between
nulland a JSON parse failure, a performance regression where negated assertions could wait for the entire timeout even when the request was clearly a non-match, a stale documentation comment, and a public TypeScript type that no longer reflected the runtime behavior.I addressed these in a second pass by narrowing the retry logic to only wait when it was actually necessary, removing redundant parsing, fixing Buffer handling, clarifying the parsing behavior, updating the documentation, and widening the public type.
The final change is committed as
a1c4ef2on thefix/toBeRequestedWith-body-matchingbranch. The test suite currently has 43 passing tests, including several tests that were verified by reverting individual fixes to confirm they actually catch the bugs they were intended to cover. There are no known regressions or additional overhead in the common case.The changes have not yet been pushed or opened as a pull request.