-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Apollo Server Integration Testsuite #14
Add Apollo Server Integration Testsuite #14
Conversation
testMatch: ['**/src/__tests__/integration.test.ts'], | ||
transform: { | ||
'^.+\\.(js|ts)$': ['babel-jest', { plugins: ['tsconfig-paths-module-resolver'], presets: ['next/babel'] }], | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make sure we stay as close to Next.js as possible I use Next.js' babel preset.
const handler = startServerAndCreateNextHandler(server, testOptions); | ||
|
||
const httpServer = createServer((req, res) => | ||
apiResolver(req, res, '', handler, {} as Parameters<typeof apiResolver>[4], false), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By reusing Next.js' apiResolver we can stay as close as possible to how Next.js parses API route requests.
…est (#7080) I'm currently building a Next.js integration for Apollo Server 4 over [here](https://github.com/apollo-server-integrations/apollo-server-integration-next). To make sure the integration works properly with Next.js I'm reusing Next.js's [apiResolver](https://github.com/vercel/next.js/blob/canary/packages/next/server/api-utils/node.ts#L426), which handles parsing API route requests. I [managed to make every test](apollo-server-integrations/apollo-server-integration-next#14) in `apollo-server-integrations-testsuite` pass except for one, `throws an error if POST JSON is malformed`. This test checks the error message in the server response when malformed JSON data is sent and matches it to two possible substrings, `Unexpected token f` and `Bad Request`. Next.js's `apiResolver` uses the [parseBody](https://github.com/vercel/next.js/blob/canary/packages/next/server/api-utils/node.ts#L143) function to parse the request body, which in turn uses [parseJSON](https://github.com/vercel/next.js/blob/canary/packages/next/server/api-utils/node.ts#L126) to parse JSON. In the case of malformed JSON `parseJSON` throws an `ApiError` with the message `Invalid JSON`, which is then returned in the server response. Since it's not possible for me to change the message returned by Next.js I was hoping I could add it to the tests's allowed error messages, making the test pass. Co-authored-by: David Glasser <glasser@apollographql.com>
@trevor-scheer Hope you had/are having a great vacation! Could you add this repository to the next team, and if possible make me a maintainer so I can merge pull requests by myself? I would also like to use GitHub Actions, but I'm not sure if we're allowed to use it in the apollo-server-integrations organisation. |
Thanks @martinnabhan, it was excellent! I've made you an admin for this repo and also added you to the next team. You should be able to use GH actions, but let me know if that's not the case and I'll see what I can turn up. They appear to be enabled for the org right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I ran the tests locally and they're all passing. Maybe just get GH actions / CI up and running in a follow up (non-fork) PR?
@@ -16,6 +16,18 @@ jobs: | |||
- run: npm ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your workflows won't run since they're on a fork - since you're admin now, you can work directly in the repository instead of in a fork.
}; | ||
}, | ||
{ | ||
serverIsStartedInBackground: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should disable the incremental delivery tests (@defer
) here as well using the noIncrementalDelivery: true
option.
@@ -6,10 +6,9 @@ import { parse } from 'url'; | |||
|
|||
interface Options<Context extends BaseContext> { | |||
context?: ContextFunction<Parameters<NextApiHandler>, Context>; | |||
started?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used anywhere?
Fixes #2
Fixes #5