-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Introduce an internal plugin test harness to facilitate plugin testing. #3990
Merged
abernix
merged 2 commits into
abernix/add-willResolveField-and-didResolveField
from
abernix/introduce-internal-plugin-testing-harness
Apr 16, 2020
Merged
Introduce an internal plugin test harness to facilitate plugin testing. #3990
abernix
merged 2 commits into
abernix/add-willResolveField-and-didResolveField
from
abernix/introduce-internal-plugin-testing-harness
Apr 16, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This test harness is meant to avoid the need to do the more heavy execution which the request pipeline itself does within `processGraphQLRequest`. I'm not prepared to make this a public-facing harness just yet, but I have reason to believe that it could be beneficial for external plugin authors to take advantage of something like this - possibly within the context of `apollo-server-plugin-base`. There's perhaps a best-of-both-worlds approach here where the request pipeline could be tested against a more precise plugin API contract, but I'm deferring that work for now.
abernix
force-pushed
the
abernix/introduce-internal-plugin-testing-harness
branch
from
April 15, 2020 18:03
79db8d8
to
ab3855d
Compare
abernix
changed the base branch from
abernix/graphql-extensions-deprecation-staging
to
abernix/add-willResolveField-and-didResolveField
April 15, 2020 18:04
trevor-scheer
approved these changes
Apr 15, 2020
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.
This is great, looks like it'll be handy!
abernix
added a commit
that referenced
this pull request
Apr 16, 2020
…o it. Similar to 6009d8a, which migrated the tracing package to the plugin API, this commit introduces a `plugin` (named) export from the `apollo-cache-control` module. It similarly accomplishes that by duplicating the behavior of the `CacheControlExtension` which leveraged the soon-to-be-deprecated `graphql-extensions`. The functionality, again, is intended to be identical in spirit. Since the delta of the commits was otherwise even _more_ confusing, I've left the `CacheControlExtension` present and exported and will remove it in a subsequent commit. Briefly summarizing what the necessary changes were: 1. We no longer use a class instance to house the extension, which was necessitated by the `graphql-extensions` API. This means that uses of `this` have been replaced with function scoped variables by the same name. 2. The logic which actually does the formatting (previously handled by the `format` method in `graphql-extension`, now takes place within the plugin API's `willSendResponse` method. 3. Rather than leaning on plugin-specific behavior from within the request pipeline, the `apollo-cache-control` plugin now makes a number of assertions throughout its various life-cycle methods to ensure that the `overallCachePolicy` is calculated. 4. Switch tests to use the new `pluginTestHarness` method for testing plugins which was introduced in eec87a6 (#3990).
abernix
added a commit
that referenced
this pull request
Apr 16, 2020
…o it. Similar to 6009d8a, which migrated the tracing package to the plugin API, this commit introduces a `plugin` (named) export from the `apollo-cache-control` module. It similarly accomplishes that by duplicating the behavior of the `CacheControlExtension` which leveraged the soon-to-be-deprecated `graphql-extensions`. The functionality, again, is intended to be identical in spirit. Since the delta of the commits was otherwise even _more_ confusing, I've left the `CacheControlExtension` present and exported and will remove it in a subsequent commit. Briefly summarizing what the necessary changes were: 1. We no longer use a class instance to house the extension, which was necessitated by the `graphql-extensions` API. This means that uses of `this` have been replaced with function scoped variables by the same name. 2. The logic which actually does the formatting (previously handled by the `format` method in `graphql-extension`, now takes place within the plugin API's `willSendResponse` method. 3. Rather than leaning on plugin-specific behavior from within the request pipeline, the `apollo-cache-control` plugin now makes a number of assertions throughout its various life-cycle methods to ensure that the `overallCachePolicy` is calculated. 4. Switch tests to use the new `pluginTestHarness` method for testing plugins which was introduced in eec87a6 (#3990).
abernix
added a commit
that referenced
this pull request
Apr 16, 2020
…o it. Similar to 6009d8a, which migrated the tracing package to the plugin API, this commit introduces a `plugin` (named) export from the `apollo-cache-control` module. It similarly accomplishes that by duplicating the behavior of the `CacheControlExtension` which leveraged the soon-to-be-deprecated `graphql-extensions`. The functionality, again, is intended to be identical in spirit. Since the delta of the commits was otherwise even _more_ confusing, I've left the `CacheControlExtension` present and exported and will remove it in a subsequent commit. Briefly summarizing what the necessary changes were: 1. We no longer use a class instance to house the extension, which was necessitated by the `graphql-extensions` API. This means that uses of `this` have been replaced with function scoped variables by the same name. 2. The logic which actually does the formatting (previously handled by the `format` method in `graphql-extension`, now takes place within the plugin API's `willSendResponse` method. 3. Rather than leaning on plugin-specific behavior from within the request pipeline, the `apollo-cache-control` plugin now makes a number of assertions throughout its various life-cycle methods to ensure that the `overallCachePolicy` is calculated. 4. Switch tests to use the new `pluginTestHarness` method for testing plugins which was introduced in eec87a6 (#3990).
This was referenced Apr 16, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This test harness is meant to avoid the need to do the more heavy execution which the request pipeline itself does within
processGraphQLRequest
when we test some of our internal plugins.I'm not prepared to make this a public-facing harness in its current state, but I have reason to believe that it could be beneficial for external plugin authors to take advantage of something like this - possibly within the context of
apollo-server-plugin-base
.There's perhaps a best-of-both-worlds approach here where the request pipeline could be tested against a more precise plugin API contract, but I'm deferring until later for now. As it stands right now, this was a requisite for doing the migrations of the
apollo-engine-reporting
andapollo-cache-control
extensions over to the new plugin API.Actual usages of this will be seen in those upcoming PRs.