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

Add createTestClient #1104

Merged
merged 4 commits into from Nov 9, 2020
Merged

Add createTestClient #1104

merged 4 commits into from Nov 9, 2020

Conversation

iddan
Copy link
Contributor

@iddan iddan commented Aug 22, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

GraphQL tests are done using supertest.

What is the new behavior?

GraphQL tests are done using apollo-server-testing

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

  • I'm not sure I placed the new function in the write place
  • I'm not sure how to update the docs regarding this
  • I only updated the minimal amount of tests for testing the function is working correctly, if you'd like I can update all the remaining tests

Resolves: #1101

feat: Use createTestClient in code-first-federation.spec.ts
feat: Use createTestClient in code-first.spec.ts
@kamilmysliwiec
Copy link
Member

Thanks for your contribution! I've left a few minor comments

iddan and others added 2 commits August 31, 2020 17:18
Co-authored-by: Kamil Mysliwiec <mail@kamilmysliwiec.com>
refactor: Move createTestClient to tests/utils
@iddan
Copy link
Contributor Author

iddan commented Aug 31, 2020

Thank you @kamilmysliwiec for your review, I've updated the code. If you can, I would like your input about these open questions:

  • How to update the docs?
  • Should I update the remaining tests to use the test client?

package.json Outdated Show resolved Hide resolved
@kamilmysliwiec
Copy link
Member

Should I update the remaining tests to use the test client?

Up to you :) It would be nice to have them consistent though

How to update the docs?

Good question. Perhaps, we could just add another section here https://docs.nestjs.com/fundamentals/testing?

@iddan
Copy link
Contributor Author

iddan commented Sep 1, 2020

Up to you :) It would be nice to have them consistent though

If it's okay I'll rather not do it right now as I'm really busy. If I'll have the time I'll create another PR later.

Good question. Perhaps, we could just add another section here https://docs.nestjs.com/fundamentals/testing?

I think that would do, though maybe it's better to mention it in the GraphQL documentation? I don't have a strong opinion on this...

Co-authored-by: Kamil Mysliwiec <mail@kamilmysliwiec.com>
@iddan
Copy link
Contributor Author

iddan commented Sep 1, 2020

Ready to merge?

@iddan
Copy link
Contributor Author

iddan commented Sep 6, 2020

@kamilmysliwiec anything left to do?

@felixveysseyre
Copy link

The tools provided by this PR would be useful to me !

@WarKaa
Copy link

WarKaa commented Nov 7, 2020

I can't wait for it to be merged. Great work @iddan

@kamilmysliwiec kamilmysliwiec merged commit d4493af into nestjs:master Nov 9, 2020
@kamilmysliwiec
Copy link
Member

Thanks! This will be added in the incoming release

PiDelport added a commit to registreerocks/registree-core that referenced this pull request Feb 8, 2021
Motivation for this update: @nestjs/graphql 7.8.0 adds getApolloServer,
which is needed for things like apollo-server-testing's
createTestClient.

* nestjs/graphql#1101
* nestjs/graphql#1104
* https://github.com/nestjs/graphql/releases/tag/7.8.0

Note:

This directly updates the same packages that "yarn nest update" updates,
though "nest update" doesn't work with Yarn 2 yet.

Invocation (for reference):

yarn up @nestjs/common @nestjs/config @nestjs/core @nestjs/graphql
@nestjs/mongoose @nestjs/passport @nestjs/platform-express
@nestjs/terminus

yarn dedupe
PiDelport added a commit to registreerocks/registree-core that referenced this pull request Feb 9, 2021
Motivation for this update: @nestjs/graphql 7.8.0 adds getApolloServer,
which is needed for things like apollo-server-testing's
createTestClient.

* nestjs/graphql#1101
* nestjs/graphql#1104
* https://github.com/nestjs/graphql/releases/tag/7.8.0

Note:

This directly updates the same packages that "yarn nest update" updates,
though "nest update" doesn't work with Yarn 2 yet.

Invocation (for reference):

yarn up @nestjs/common @nestjs/config @nestjs/core @nestjs/graphql
@nestjs/mongoose @nestjs/passport @nestjs/platform-express
@nestjs/terminus

yarn dedupe
PiDelport added a commit to registreerocks/registree-core that referenced this pull request Feb 9, 2021
Motivation for this update: @nestjs/graphql 7.8.0 adds getApolloServer,
which is needed for things like apollo-server-testing's
createTestClient.

* nestjs/graphql#1101
* nestjs/graphql#1104
* https://github.com/nestjs/graphql/releases/tag/7.8.0

Note:

This directly updates the same packages that "yarn nest update" updates,
though "nest update" doesn't work with Yarn 2 yet.

Invocation (for reference):

yarn up @nestjs/common @nestjs/config @nestjs/core @nestjs/graphql
@nestjs/mongoose @nestjs/passport @nestjs/platform-express
@nestjs/terminus

yarn dedupe
@DonnyVerduijn
Copy link

Yes. It is exported from index.ts

It's not part of the build when it's installed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate with apollo-server-testing
6 participants