Skip to content
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

Allows resolvers to be used with mocks #9

Merged
merged 1 commit into from
May 26, 2016
Merged

Conversation

rdrey
Copy link
Contributor

@rdrey rdrey commented May 26, 2016

Previously mocks would overwrite any resolvers. Includes test. As discussed in ardatan/graphql-tools#53.

Previously mocks would overwrite any resolvers. Includes test.
@apollo-cla
Copy link

@rdrey: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@coveralls
Copy link

coveralls commented May 26, 2016

Coverage Status

Coverage remained the same at 93.827% when pulling 2f5ced2 on rdrey:master into 2a4a419 on apollostack:master.

@rdrey
Copy link
Contributor Author

rdrey commented May 26, 2016

Once this is merged we need to update http://docs.apollostack.com/apollo-server/tools.html#mocks

The first sentence could be:

If provided, mocks will mock the results of the GraphQL query for any resolve functions not defined on the schema.

@helfer
Copy link
Contributor

helfer commented May 26, 2016

Thanks, this looks good! I like the sentence you wrote, can you add that to the docs?

@helfer helfer merged commit a056dd2 into apollographql:master May 26, 2016
rdrey pushed a commit to rdrey/docs that referenced this pull request May 26, 2016
Documentation update for [Allows resolvers to be used with
mocks](apollographql/apollo-server#9)
@sebastienbarre
Copy link

I'm having issue with this one (version 0.2). Or maybe I'm not understanding what it does correctly. Does it imply that I don't need to have all my resolvers defined, as long as they are mocked? Which would be great.
Right now, I'm getting an error from makeExecutableSchema if any resolver is missing:

Error: Resolve function missing for "Query.employee"

even though I intend on mocking it later on when calling addMockFunctionsToSchema.

My resolvers.js is more or less:

const resolvers = {
  // Query: {
  //   employee(_, args) {
  //     return Employee.find({ where: args });
  //   },
  // },
};

and my mock.js:

const mocks = {
  Query: () => ({
    employee: (root, args) => {
      return { first_name: args.first_name, last_name: args.last_name };
    },
  }),
  Employee: () => ({ first_name: () => casual.first_name, last_name: () => casual.last_name }),

Thanks guys

@sebastienbarre
Copy link

OK, after digging in the doc, it appears a few more options are needed when calling makeExecutableSchema beforehand, for this scenario to work:

const executableSchema = makeExecutableSchema({
  typeDefs: Schema,
  resolvers: Resolvers,
  allowUndefinedInResolve: true,
  resolverValidationOptions: {
    requireResolversForArgs: false,
    requireResolversForNonScalar: false,
  },
});
...
addMockFunctionsToSchema({
  schema: executableSchema,
  mocks: Mocks,
  preserveResolvers: true,
});

I can submit a PR to update the Mocking page accordingly, if needed. It's not entirely related to preserveResolvers but this seems to be the only way to add mocks using addMockFunctionsToSchema (and without mockServer). Otherwise makeExecutableSchema will always complain about missing resolvers (that are meant to be mocked). Am I missing something?

@rdrey
Copy link
Contributor Author

rdrey commented Aug 4, 2016

You're using the lower-level functions manually. This PR was for apolloServer() specifically, which lets you mix resolvers and mocks. You're right that you need to set preserveResolvers: true, etc, if you chain the functions together yourself. :)

@sebastienbarre
Copy link

Sorry, I'm not sure I follow. I'm looking at both the docs for appolloServer setup, and the definition of apolloOptions, and I do not see how you can pass mocks there. It seems the constructor only accept an executable GraphQL schema, so my assumption is that makeExecutableSchema and addMockFunctionsToSchema always need to be called to make the schema executable and specify mocks. Correct? Is there a different way to create and mock the server, without calling the low-level functions manually?

@rdrey
Copy link
Contributor Author

rdrey commented Aug 4, 2016

Ah sorry, I haven't had time to play with Apollo in a while.

When I sent this PR those setup methods didn't exist yet, I've been constructing apolloServer like this in the past:

const graphQLServer = express();

graphQLServer.use('/graphql', apolloServer({
  graphiql: true,
  pretty: true,
  schema: Schema,
  resolvers: Resolvers,
  //mocks: Mocks,
}));

stolen from http://docs.apollostack.com/graphql-tools/guide.html

When using that constructor the Schema doesn't need to be an executable Schema, it will take just the string version. (And allows you to mix resolvers & mocks.)

I haven't checked out the new constructors (Express, Connect, HAPI, Koa, etc) to see if they support this.

@sebastienbarre
Copy link

@rdrey thanks, I understand now. This is now done differently in 0.2.

@helfer
Copy link
Contributor

helfer commented Aug 4, 2016

@sebastienbarre Yes, Apollo Server 0.2 is quite different from 0.1 because it doesn't include graphql-tools any more. If the migration guide doesn't work as expected with mocking, a PR would be greatly appreciated!

@sebastienbarre
Copy link

sebastienbarre commented Aug 4, 2016

@helfer thanks. I'm still playing around with Apollo, let me push something once I'm a bit more confident I know my way around :)

@rdrey @helfer There might be a bug in preserveResolvers or the way mocks override resolvers though...
Consider that schema:

type Employee {
  first_name: String
  last_name: String
  projects: [Project]
}
type Project {
  name: String
}
type Query {
  employee(first_name: String, last_name: String): Employee
}

and the resolvers:

const resolvers = {
  Query: {
    employee(_, args) {
      return Employee.find({ where: args });
    },
  },
}

where the Query.employee resolver returns a promise that queries a live MySQL database and return an object with the first_name and last_name properties BUT NOT any projects property yet.

Now consider that mock:

import casual from 'casual';
const mocks = {
  Employee: () => ({
    first_name: () => casual.first_name,
    last_name: () => casual.last_name,
    projects: () => [{ name: 'foo' }, { name: 'bar' }],
  }),
  Project: () => ({
    name: casual.title,
  }),
};

if you query:

employee(first_name:"Sebastien") {
    first_name
    last_name
    projects {
      name
    }
  }
}

it will return:

{
  "data": {
    "employee": {
      "first_name": "Sebastien",
      "last_name": "Barre",
      "projects": [
        {
          "name": "Autem omnis officiis"
        },
        {
          "name": "Voluptas consequatur autem"
        }
      ]
    }
  }
}

Assuming the MySQL record for { first_name: 'Sebastien', last_name: 'Barre'} is in the database.

That does not seem correct though. What I'm trying to do here is: "OK, I've the correct MySQL resolver for the employee, but I haven't really nailed down how to return all the projects associated to an employee from the MySQL database, so I want to mock them instead".

However it seems that the mock Employee.projects is ignored. My assumption was that:

  • once the Query.employee resolver was called, only the first_name and last_name fields were returned, not the projects field,
  • therefore the server, knowing that Query.employee is supposed to return an Employee type, would look for the projects property in the Employee mock,
  • which would have returned [{ name: 'foo' }, { name: 'bar' }].

I'm not sure why the Employee.projects mock is being ignored and why the Project mock is invoked instead, creating fake names for an array that always has 2 elements (I assume that's the mock framework picking that length). However, I want to control the number of elements and name in the array returned by Employee.projects...

Thanks
I realize this is more a graphql-tools issue though, to be found in mock.js

@helfer
Copy link
Contributor

helfer commented Aug 5, 2016

@sebastienbarre The mocks aren't always very intuitive to use, especially when mixed with real resolvers. What is happening in your case is that graphql-tools sees that Employee does not need to be mocked because it has a real resolve function. It thus never applies the Employees.project mock function. When the employee returned doesn't have a project, graphql-tools applies the Project mock function, in which you have defined name as casual.title.

If you wanted graphql-tools to apply the Employee.projects mock function that you have defined, you would have to submit a PR that does that (or as a first step, submit a PR with a failing test for your specific case). I don't think it would be too hard, but as it is currently implemented it doesn't do what you expect.

@sebastienbarre
Copy link

sebastienbarre commented Aug 6, 2016

@helfer Thanks. The mocks are a great feature, thanks for all the good work. I'm pushing a PR with a failing test.

jbaxleyiii pushed a commit to apollographql/apollo-client that referenced this pull request Oct 17, 2017
Documentation update for [Allows resolvers to be used with
mocks](apollographql/apollo-server#9)
martijnwalraven pushed a commit that referenced this pull request Jun 27, 2018
* dev: Update TypeScript to latest version, v2.7.2.

* dev: Update `graphql` to latest version, v0.13.2.

* dev: Update jest & dependencies to latest versions.

* dev: Update type definitions for `graphql`, `node` and `jest`.
evans pushed a commit that referenced this pull request Jun 27, 2018
…rver-env (#1259)

* Export polyfills and types separately

* More imports from apollo-server-env

* Initial commit

* Add .npmignore to avoid ignoring lib when publishing

* 0.0.2

* Reorganize code and clean up GraphQLExtension interface

* 0.0.3

* Add support for timing callbacks and add GraphQLExtensionStack

* 0.0.4

* Downgrade target in tsconfig.json from es2015 to es5

* 0.0.5

* Bump `graphql` peerDependency. (#3)

* 0.0.6

* Update dependencies

* 0.0.7

* whenResultIsFinished fix for array results (#4)

* 0.0.8

* [apollo-bot] Update the Issue/PR Templates with auto label (#6)

* Bump `graphql` peerDependency. (#7)

* Update `graphql` peer dependency range to allow 0.13.x. (#8)

* Update `devDependencies` to latest versions. (#9)

* dev: Update TypeScript to latest version, v2.7.2.

* dev: Update `graphql` to latest version, v0.13.2.

* dev: Update jest & dependencies to latest versions.

* dev: Update type definitions for `graphql`, `node` and `jest`.

* Allow `undefined` return values to `GraphQLExtension`'s `format()`. (#10)

In some cases, it's conceivable that the `format()` method may need to abort
its decision to provide extension information at runtime, in the event that
it doesn't have the proper information to return a full-result.

The `format` method already removed false-y results, so this simply changes
the types to allow the same.

* 0.0.9

* Fix lifecycle method invocations on extensions

* 0.0.10

* Add changelog

* Upgrade to TypeScript 2.8

Makes my editor integration happier (a bugfix in tsserver I think)

* Add tslint and prettier

Same configuration as apollo-engine-js

* Remove magic from GraphQLExtensionStack constructor

It's not hard to consistently pass in an actual extension object to this
low-level API.

* New extension API: didStart handlers return didEnd handlers

This is a backwards-incompatible change: GraphQLExtension implementations and
users of GraphQLExtensionStack (ie apollo-server-core) must change their
implementations, if they implement any of the xDidStart/xDidEnd APIs.

This allows "didEnd" handlers to refer to closure variables from the "didStart"
handler rather than needing to store state on the extension.

The new "didEnd" handlers run in the opposite order of the "didStart" handlers,
so that they properly nest.

* 0.1.0-beta.0

* Changelog

* Add magic back into GraphQLExtensionStack constructor

But now it actually gets more context (the execution arguments) and doesn't have
to be a constructor.

* 0.1.0-beta.1

* Export more types

* 0.1.0-beta.2

* Fix lifecycle handlers to pass proper "this"

* 0.1.0-beta.3

* Pass options directly to start handlers; eliminate factory again

* 0.1.0-beta.4

* error handling in didEnd

* 0.1.0-beta.5

* pass multiple errors to EndHandler

* 0.1.0-beta.6

* add willSendResponse

* 0.1.0-beta.7

* prettier

* setFieldResolver for custom fieldResolver

* reverse

* get more initial options into requestDidStart

* 0.1.0-beta.8

* 0.1.0-beta.9

* Actually, we already get the fieldResolver!

* 0.1.0-beta.10

* work without extensionStack

* 0.1.0-beta.11

* 0.1.0-beta.12

* Send errors to willResolveField callback

* 0.1.0-beta.13

* willSendResponse can return a result

* 0.1.0-beta.14

* Revert 1063be8..56912fc

This reverts commit 1063be8..56912fc.

* add PQ options to requestDidStart

* 0.1.0-beta.14

* 0.1.0-beta.15

* Initialize an empty TypeScript/Jest package

Template based on apollo-engine-js

* Basic trace node structure building

* basic timing

* Checkpoint towards signature implementation

The new signature implementation does not try to compress whitespace.

* Basic signature implementation

* progress towards actual reporting

* basic checkpoint for reporting

* 0.0.0-beta.1

* pull in @types/long, since it is in the external api

* 0.0.0-beta.2

* get rid of Long

* 0.0.0-beta.3

* debug log request what happened

* 0.0.0-beta.4

* 0.0.0-beta.5

* correct url

* 0.0.0-beta.6

* request headers

* 0.0.0-beta.7

* leave out a few headers

* 0.0.0-beta.8

* prettier

* move stuff into multiple files, and stop exporting the extension

* lots of doc comments

* address agent.ts XXX comments

* implement privateVariables

simplify API by removing flush() and allowing flush-ers to just call sendReport
directly

* privateHeaders and error tracking

* gzip, signals

* fix test

* 0.0.0-beta.9

* Error handling for reports

* 0.0.0-beta.10

* no need to include boring stacktrace

* 0.0.0-beta.11

* tweak error reporting

* 0.0.0-beta.12

* package-lock update (npm@6?)

* Reduce target report size to 512KB from 4MB.

Load testing revealed that protobuf encoding for large FullTraceReports could
tie up CPU and reduce p99 request latency (eg, to 200ms from 10ms). Reducing the
default target report size spreads out the encoding time and mitigates the
impact on latency.  If this is not acceptable for all users, we may have to
investigate reintroducing agent-side stats aggregation to keep report sizes
small.

* 0.0.0-beta.13

* Encode Traces as they come in

This improves p99 times with little effect on p50 times. It also lets us get rid
of the heuristic average trace size estimation.

* 0.0.0-beta.14

* support PQ fields

* npm audit fix

* 0.0.0-beta.15

* ignore coverage

* Make the default signature more aggressive

We'd rather tell people confused by literal removal to tweak the signature than
tell people causing outages to do so.

* 0.0.0-beta.16

* Remove obsolete files from graphql-extensions and apollo-engine-reporting

* Fix dependencies and configs

* Fix apollo-server-cloudflare to import from apollo-server-env

* Fix compilation and test configs

* Get all tests passing again

* Switch to Lerna independent versioning

* Polyfill promisify for Node < 8 and load polyfills in tests

* ES2016 exponentiation operator is only supported in Node > 6

* add dependency cache for circle

* add missing env dependencies in REST datasource
evans added a commit that referenced this pull request Jul 3, 2018
* Initial commit

* 0.0.3

* Replace endOffset with duration in resolver calls

* 0.0.4

* Fix duration

* 0.0.5

* Remove unnecessary schema level resolve function and return schema

* Update README

* 0.0.6

* Update README

* Update dependencies

* 0.0.7

* Update README

* set package.json to point to this repository (#3)

* Update dependencies

Fixes #4.

* 0.0.8

* Add asynciterable support to tsconfig.json

* Skip trace collection when context or context._traceCollector is undefined

Fixes #5.

* 0.0.9

* Rewrite to use graphql-extensions

* 0.0.10

* 0.0.11

* 0.1.0

* Update graphql-extensions dependency and downgrade TS target

* 0.1.1

* Update README

* Update README

* Increase version range for `graphql` peerDependency. (#7)

* 0.1.2

* Update dependencies

* 0.1.3

* [apollo-bot] Update the Issue/PR Templates with auto label (#9)

* Update `graphql` peer dependency range to allow 0.13.x.

* dev: Update TypeScript to latest version, v2.7.2.

* dev: Update jest & dependencies to latest versions.

* dev: Update type definitions for `graphql`, `node` and `jest`.

* Allow `undefined` to return from `format`. (#12)

* Allow `undefined` to return from `format`.

TypeScript 2.7 introduced new "Strict Class Instantiation" rules which,
as the name suggests, require properties which are intended to be set
(eventually, to a type) be set during construction.

Before this change, the `TracingExtension` class was deferred setting these
private properties (namely, `startWallTime`, `endWallTime`, `startHrTime`
and `duration`), but not during instantiation which required setting
marking them as optional and guarding their usage in other methods which
might use those (temporarily `undefined`, if even for a tick) properties.
For example, the expectation that `format` is _only_ called after
`requestDidStart` is not guaranteed with this configuration, even if it is
expected under normal operation.

Therefore, this change adds the additional guarding and updates the `format`
method to return `undefined` in the event that it doesn't have the appropriate
data.

* Update `graphql-extensions` dependency to `~0.0.9`.

Specifically, to take advantage of a type which landed in
`[email protected]` thanks to
https://github.com/apollographql/graphql-extensions/pull/10.

* 0.1.4

* Update for [email protected] API (#13)

* Upgrade to TypeScript 2.8

* Add tslint and prettier

* Update for [email protected] API

* 0.2.0-beta.0

* Make work with newest API usage

format() now gets called before the requestDidStart() EndHandler.

* 0.2.0-beta.1

* remove unused files from tracing package

* upgrade packages, fix compilation bugs, and add test
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants