Open rmclaughlin-nelnet opened 2 years ago
Hi @rmclaughlin-nelnet, thanks for raising this! 😃
I've been meaning to support node-fetch
for a while as it's pretty popular.
What's the error you get when passing a node-fetch
response to the toSatisfyApiSpec
assertion?
I expect the solution will be straightforward:
node-fetch
responses hereNodeFetchResponse
class, similar to the AxiosResponseClass
node-fetch
responses from responses of other request packages.I'll do this when I have time, if you want it sooner you're welcome to make a pull request
Thanks for the response. Here is the error I get
TypeError: Cannot read property 'path' of undefined
160 | });
161 |
> 162 | expect(res).toSatisfyApiSpec();
| ^
163 | });
164 | });
165 |
at Object.getPathname (node_modules/openapi-validator/lib/utils/common.utils.ts:13:21)
at OpenApi3Spec.findExpectedPathItem (node_modules/openapi-validator/lib/classes/AbstractOpenApiSpec.ts:106:28)
at OpenApi3Spec.findExpectedResponseOperation (node_modules/openapi-validator/lib/classes/AbstractOpenApiSpec.ts:113:33)
at OpenApi3Spec.findExpectedResponse (node_modules/openapi-validator/lib/classes/AbstractOpenApiSpec.ts:80:44)
at OpenApi3Spec.validateResponse (node_modules/openapi-validator/lib/classes/AbstractOpenApiSpec.ts:123:31)
at Object.default_1 (node_modules/jest-openapi/src/matchers/toSatisfyApiSpec.ts:25:39)
at Object.toSatisfyApiSpec (node_modules/jest-openapi/src/index.ts:28:31)
at __EXTERNAL_MATCHER_TRAP__ (../../node_modules/expect/build/index.js:386:30)
at Object.throwingMatcher [as toSatisfyApiSpec] (../../node_modules/expect/build/index.js:387:15)
at Object.<anonymous> (e2e.test.js:162:17)
Thanks, also which version of node-fetch
are you using? Looks like most people are on 2.6.5:
Yes, we are on 2.6.x
hey, so node-fetch
(2.6.5) is trickier to support than I expected because its response
object (i.e. the result of await fetch(...)
) is different to that of the request packages we already support (axios, superagent, request-promise). I found a workaround, see below. Let me know if it solves your immediate problem, and perhaps we can improve it or expose a new async matcher.
Problem
Firstly there is no sync method to access the node-fetch response body. It must be accessed via either await response.json()
or await response.text()
(depending on the header). So expect(await fetch(...)).toSatisfyApiSpec()
won't work, we'll have to expose an asynchronous method (expect(response).toEventuallySatisfyApiSpec()
?).
Secondly, the response does not include the original request method. We use this to find which OpenAPI response definition the actual node-fetch
response should satisfy. So the matcher might have to be expect(response).toEventuallySatisfyApiSpec('GET')
.
Workaround (corrected on 30 Oct) This works by parsing the response into a superagent-like response first:
describe('test', () => {
it('passes', async () => {
const rawResponse = await fetch('endpoint');
const body = await rawResponse.json();
const res = {
req: {
path: rawResponse.url,
method: 'GET',
},
status: rawResponse.status,
body,
text: JSON.stringify(body),
};
expect(res).to.satisfyApiSpec;
});
});
If you need to do lots of these, you can automate the parsing:
type HTTPMethod =
| 'GET'
| 'POST'
| 'PUT'
| 'DELETE'
| 'OPTIONS'
| 'HEAD'
| 'PATCH';
type ParsedResponse = {
req: {
path: string;
method: HTTPMethod;
};
status: number;
body: unknown;
text: string;
};
const parseResponse = async (
rawResponse: NodeFetchResponse,
requestMethod: HTTPMethod,
): Promise<ParsedResponse> => {
const isJson = rawResponse.headers.get('content-type')?.includes('json');
const req = {
path: rawResponse.url,
method: requestMethod,
};
const { status } = rawResponse;
if (isJson) {
const body = await rawResponse.json();
const text = JSON.stringify(body);
return {
req,
status,
body,
text,
};
}
const text = await rawResponse.text();
return {
req,
status,
body: {},
text,
};
};
describe('test', () => {
it('passes', async () => {
const rawResponse = await fetch('endpoint');
const res = await parseResponse(rawResponse, 'GET');
expect(res).to.satisfyApiSpec;
});
});
Longer term solution A new async assertion?
describe('test', () => {
it('passes', async () => {
const method = 'GET';
const res = await fetch('endpoint', { method });
await expect(res).toEventuallySatisfyApiSpec(method);
});
});
Or, expose the above parseResponse
helper function as parseNodeFetchResponse
.
Thanks for looking into it. the first solution should be good enough for me, but I agree a longer term solution would be better. I was trying out the first solution, but I ran into problems. Here is what I did, slightly different from what you typed out.
const rawResponse = await fetch('endpoint');
const body = await rawResponse.json();
const res = {
req: {
path: rawResponse.url,
method: 'POST',
},
body,
text: JSON.stringify(body),
};
expect(res).toSatisfyApiSpec();
TypeError: Cannot read property '_json' of undefined
98 | text: JSON.stringify(body),
99 | };
> 100 | expect(res).toSatisfyApiSpec();
| ^
101 | });
102 | });
103 |
Ah, sorry I miswrote the first solution slightly, you need the status
field so that we parse the response as a SuperAgent response, rather than a RequestPromise response (hence why it expected _json
)
I've corrected the above example, so it should work when a response header specifies the content type is json.
If instead the content type is text, use this:
describe('test', () => {
it('passes', async () => {
const rawResponse = await fetch('endpoint');
const text = await rawResponse.text();
const res = {
req: {
path: rawResponse.url,
method: 'GET',
},
status: rawResponse.status,
body: {},
text,
};
expect(res).to.satisfyApiSpec;
});
});
Let me know if these work, and we can find a better long-term solution
Ok, I got it to work with a few tweaks
const rawResponse = await fetch('endpoint');
const body = await rawResponse.json();
const res = {
req: {
path: rawResponse.url,
method: 'GET',
},
status: rawResponse.status,
body,
};
expect(res).toSatisfyApiSpec();
expect(res).to.satisfyApiSpec;
but I had to use expect(res).toSatisfyApiSpec();
Thanks! In answer to your points:
text
is unnecessaryadditionalProperties: false
and/or required
properties
OpenAPI version 2 and/or 3? 3 Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] This looks like a great solution to a problem we have been facing. However, we use node-fetch exclusively in our code and tests. Do you have any plans to support node-fetch? Describe the solution you'd like A clear and concise description of what you want to happen. Support for node-fetch
Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.
Additional context Add any other context or screenshots about the feature request here.
Are you going to resolve the issue?