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

gateway: stop using apollo-server-testing #554

Merged
merged 1 commit into from Mar 9, 2021

Conversation

glasser
Copy link
Member

@glasser glasser commented Mar 8, 2021

I've been making the assertion in various issues on apollo-server that I don't
feel like apollo-server-testing has much value over using the
executeOperation method (which was added to AS as part of implementing
apollo-server-testing) directly. I figured it was worth trying out my
suggestion over on this repo (esp when I noticed #553 upgrading
apollo-server-testing). My opinion is that the version without
createTestClient is more clear in these particular tests in that adding an
intermediate call object only serves to obfuscate.

The one nice thing that createTestClient does is allow you to pass an AST
rather than a string for the operation. We could change executeOperation to
support this directly too, though in this case just changing the tests to not
create AST nodes was easy enough.

@glasser glasser requested review from trevor-scheer and martijnwalraven and removed request for trevor-scheer March 8, 2021 21:33
Copy link
Contributor

@martijnwalraven martijnwalraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used apollo-server-testing before myself, but looking at its API and the changes in this PR I agree using executeOperation directly seems like a cleaner approach.

I've been making the assertion in various issues on `apollo-server` that I don't
feel like `apollo-server-testing` has much value over using the
`executeOperation` method (which was added to AS as part of implementing
`apollo-server-testing`) directly. I figured it was worth trying out my
suggestion over on this repo (esp when I noticed #553 upgrading
`apollo-server-testing`). My opinion is that the version without
`createTestClient` is more clear in these particular tests in that adding an
intermediate `call` object only serves to obfuscate.

The one nice thing that `createTestClient` does is allow you to pass an AST
rather than a string for the operation. We could change `executeOperation` to
support this directly too, though in this case just changing the tests to not
create AST nodes was easy enough.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants