apollo-server-core: pass req obj to context when invoking executeOperation

[vitorbal]

Fixes #2277.

apollo-server-testing relies on apollo-server-core to execute queries for integration tests, via the executeOperation method.

However, when executing a query via executeOperation, apollo-server-core was not passing the req object to the context callback function used to create the server.

This is fine if you're using a mock context object directly in your tests, but for tests that want to run a context callback that contains logic that depends on that req object, it would fail.

Note that long-term the best fix for this is probably that apollo-server-testing should use the ApolloServer class from apollo-server-express internally, instead of ApolloServerBase from apollo-server-core, since it seems like that is the class that is used by default in apollo-server.

TODO:

• Update CHANGELOG.md with your change (include reference to issue & this PR)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[vitorbal]

Nevermind, I just noticed that the req object expected there is a Request express object, and this fix would pass it GraphQLRequest object instead.

I wonder if it makes sense to create a separate testing package specifically for apollo-server-express 🤔

[lnmedrano]

Hello @vitorbal !
I think in test environment we don't expect an Express request because we are not creating any Express server. What is wrong about passing a GraphQLRequest instead? Maybe it could be a problem if some specific part of the Express is required, but in test-environment we are never getting an Express server. I support your solution to this problem and I think you should re-open the PR. Otherwise we must set the context inside of the tests.

[vitorbal]

@lnmedrano if you look at the implementation of ApolloServer, you will notice that the listen instance method creates an express server for you behind the scenes:
https://github.com/vitorbal/apollo-server/blob/93909b41727bec5a39b2d3ebc7c2e6e50249dbbb/packages/apollo-server/src/index.ts#L86

So there are basically two ways to create an apollo-server:
a) integrate it into an existing server, via apolloServer.applyMiddleware
b) standalone, via apolloServer.listen()

With a), your integration tests will need to pass to the context function the appropriate req object applicable to the server library your using. For instance, if you're using apollo-server-express, you would receive an express.Request type. If you're using Koa, you would receive an Koa.Context request type.

with b), since the server created for you uses express behind the scenes, context will also be receiving an express.Request type.

So to summarize, the type of the req and res objects that get passed into the context function depend on the server integration you're using.

Now let's talk about GraphQLRequest: GrapQLRequest is not a super type of the object that gets passed into the context function –– it's actually a different object. Take a look at its definition.

The executeOperation method I modify in this PR receives the GraphQLRequest as an argument, and not the server request we're really interested in. So we would be passing the wrong object into the context function if we do this.

Hopefully my explanation makes sense! Let me know if I missed anything.

[vitorbal]

Also, @lnmedrano (or anyone else reading this), since closing this PR, I've written a new package that mimics the apollo-server-testing API, but allows for passing in a mock req object (or sets one for you automatically, with sensible defaults):

https://github.com/zapier/apollo-server-integration-testing

We've been using it successfully at my company for the last 6 months to write real integration tests. Posting it here in case anyone else is interested in giving it a try :)