feat: enhance response matchers with utility functions and type definitions #56
Conversation
…itions Signed-off-by: GitHub <[email protected]>
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @mcansh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on enhancing the vitest-response-matchers library by introducing shared utility functions to streamline common validation and data extraction tasks. These changes lead to a more robust and type-safe codebase, while also refining the behavior of specific matchers to be more accurate and flexible in their assertions.
Highlights
- Refactoring & Utility Functions: Introduced two new utility functions,
getHeadersandverifyResponse, to centralize common logic such as extracting headers from response objects and validating if an input is an instance ofResponse. These utilities are now integrated across various matchers, significantly reducing code duplication and improving maintainability. - Type Definition Enhancements: Updated the type definition for the
toThrowResponsematcher inclient.tsto be more precise, now acceptingResponse | { status: number; statusText: string }. This provides clearer expectations for the structure of thrown response errors. - Matcher Behavior Refinements: Adjusted the
toHaveBodymatcher to considernullas a valid body, changing its internal logic fromresponse.body !== nulltotypeof response.body !== "undefined". ThetoHaveCookiesmatcher also received enhancements for more robust handling of missingSet-Cookieheaders and improved logic for cases where no cookies are expected.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packages | 91f9ea9 | Commit Preview URL Branch Preview URL |
Jul 31 2025, 06:52 PM |
There was a problem hiding this comment.
Code Review
This pull request enhances response matchers with utility functions and type definitions. However, the changes introduce potential regressions, particularly in the verifyResponse utility function and its usage, as well as logic errors in toHaveBody and toThrowResponse. Addressing these issues is crucial to ensure the matchers function correctly.
|
|
||
| return { | ||
| message: () => `Expected response to have body`, | ||
| pass: response.body !== null, | ||
| pass: typeof response.body !== "undefined", |
There was a problem hiding this comment.
The check typeof response.body !== "undefined" is not the correct way to determine if a Response has a body. The body property on a Response object is either a ReadableStream or null, but it's never undefined. This means the current check will always evaluate to true.
The correct way to check for the presence of a body stream is to check if it's not null.
| pass: typeof response.body !== "undefined", | |
| pass: response.body !== null, |
| pass: false, | ||
| }; | ||
| } | ||
| verifyResponse(response); |
There was a problem hiding this comment.
The call to verifyResponse(response) is incorrect because its return value is ignored. This effectively removes the runtime type check that was present in the previous version of the code.
Assuming verifyResponse function returns a MatcherResult on failure and undefined on success, it should be used like this to properly handle the validation:
const verification = verifyResponse(response);
if (verification) {
return verification;
}This pattern should be applied to all other matchers where this refactoring was performed.
| pass: | ||
| error instanceof Response && | ||
| error.status === expectedResponse.status && | ||
| error.statusText === expectedResponse.statusText, |
There was a problem hiding this comment.
Removing the error instanceof Response check can lead to runtime errors. It introduces two potential problems:
- If the
received()function does not throw an error,errorwill beundefined. Accessingerror.statuswill then cause aTypeError. - If a non-
Responseerror is thrown, the checks will be performed on an object that doesn't havestatusorstatusTextproperties, leading to incorrect results.
The instanceof check is essential for ensuring the thrown object is of the correct type before accessing its properties.
| pass: | |
| error instanceof Response && | |
| error.status === expectedResponse.status && | |
| error.statusText === expectedResponse.statusText, | |
| pass: | |
| error instanceof Response && | |
| error.status === expectedResponse.status && | |
| error.statusText === expectedResponse.statusText, |
| export function verifyResponse(response: unknown) { | ||
| return { | ||
| message: () => `Expected a Response, but received ${typeof response}`, | ||
| pass: !(response instanceof Response), | ||
| }; | ||
| } |
There was a problem hiding this comment.
The verifyResponse utility function has a potential flaw:
The pass property is set to !(response instanceof Response). This means if a non-Response object is passed (a failure case), pass becomes true. For Vitest matchers, pass: true indicates a successful assertion, which contradicts the failure message.
A better approach would be for this function to return a MatcherResult object on failure and undefined on success. The calling matcher can then check the return value and fail early if needed.
| export function verifyResponse(response: unknown) { | |
| return { | |
| message: () => `Expected a Response, but received ${typeof response}`, | |
| pass: !(response instanceof Response), | |
| }; | |
| } | |
| export function verifyResponse(response: unknown): { pass: false; message: () => string } | undefined { | |
| if (!(response instanceof Response)) { | |
| return { | |
| message: () => `Expected a Response, but received ${typeof response}`, | |
| pass: false, | |
| }; | |
| } | |
| } |
Signed-off-by: GitHub [email protected]