Skip to content

Integration testsuite direct dependency on Apollo Server #7114

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

Merged
merged 8 commits into from
Nov 1, 2022

Conversation

trevor-scheer
Copy link
Contributor

@trevor-scheer trevor-scheer commented Nov 1, 2022

The peer dependency arrangement of testsuite on server was problematic. In one sense, it seems reasonable since we want integration authors to bring their own AS package. However, bumping that peer dependency with every version update is technically a breaking change - and our release tooling (changeset) doesn't provide us a means to workaround the behavior where it major version bumps both packages.

For correctness and compliance with our tooling, a direct dependency addresses both concerns. We've also added an additional test which ensures that the versions match. The test really just validates that there's one install of @apollo/server (by using an instanceof check against the testsuite's ApolloServer constructor and the actual instance provided by the testsuite consumer).

Fixes #7109

The peer dependency arrangement of testsuite on server was problematic.
In one sense, it seems reasonable since we want integration authors to
bring their own AS package. However, bumping that peer dependency with
every version update is technically a breaking change - and our release
tooling (changeset) doesn't provide us a means to workaround the
behavior where it major version bumps both packages.

For correctness and compliance with our tooling, a direct dependency
addresses both concerns. We've also added an additional test which
ensures that the versions match. The test really just validates that
there's one install of @apollo/server (by using an instanceof check
against the testsuite's ApolloServer constructor and the actual instance
provided by the testsuite consumer).
@trevor-scheer trevor-scheer requested a review from glasser November 1, 2022 18:24
@netlify
Copy link

netlify bot commented Nov 1, 2022

Deploy Preview for apollo-server-docs ready!

Name Link
🔨 Latest commit 1327636
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/63616ce66593ac00085667b3
😎 Deploy Preview https://deploy-preview-7114--apollo-server-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@glasser
Copy link
Member

glasser commented Nov 1, 2022

Add a "Fixes #7109"?

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 1, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1327636:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@@ -28,6 +28,7 @@
"dependencies": {
"@apollo/cache-control-types": "^1.0.2",
"@apollo/client": "^3.6.9",
"@apollo/server": "^4.0.5",
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be precise rather than caret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

// This test ensures that consumers of this testsuite are using the lockstep
// versioned packages of @apollo/server and
// @apollo/server-integration-testsuite. If these versions are out of sync,
// this test should fail.
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if the comment and/or test name could be more clear/verbose here. Does saying something like "It looks like you have multiple copies of @apollo/server installed in your project. @apollo/server-integration-testsuite always installs a copy of @apollo/server with a precisely matching version number; have you installed @apollo/server and @apollo/server-integration-testsuite with non-matching versions?" help? (I don't know how to print custom error messages though... I guess just manually throwing an Error.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, throwing an error here now (but still running the expect for the known-passing case).

@trevor-scheer trevor-scheer marked this pull request as ready for review November 1, 2022 18:47
@trevor-scheer
Copy link
Contributor Author

To validate the case where this new test fails, I:

  • Pushed a commit with newer versions of server and testsuite packages
  • Installed the codesandbox build of testsuite into the koa integration repo
  • npm ls @apollo/server to observe the non-deduped @apollo/server packages indicating a version mismatch
  • Run npm test and note the instanceof test failure

@trevor-scheer trevor-scheer merged commit c1651bf into main Nov 1, 2022
@trevor-scheer trevor-scheer deleted the trevor/fix-dep-arrangement branch November 1, 2022 19:07
glasser pushed a commit that referenced this pull request Nov 2, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @apollo/server-integration-testsuite@4.1.0

### Minor Changes

-
[`2a2d1e3b4`](2a2d1e3)
Thanks [@glasser](https://github.com/glasser)! - The `cache-control`
HTTP response header set by the cache control plugin now properly
reflects the cache policy of all operations in a batched HTTP request.
(If you write the `cache-control` response header via a different
mechanism to a format that the plugin would not produce, the plugin no
longer writes the header.) For more information, see [advisory
GHSA-8r69-3cvp-wxc3](GHSA-8r69-3cvp-wxc3).

-
[`2a2d1e3b4`](2a2d1e3)
Thanks [@glasser](https://github.com/glasser)! - Plugins processing
multiple operations in a batched HTTP request now have a shared
`requestContext.request.http` object. Changes to HTTP response headers
and HTTP status code made by plugins operating on one operation can be
immediately seen by plugins operating on other operations in the same
HTTP request.

-
[`2a2d1e3b4`](2a2d1e3)
Thanks [@glasser](https://github.com/glasser)! - New field
`GraphQLRequestContext.requestIsBatched` available to plugins.

- [#7114](#7114)
[`c1651bfac`](c1651bf)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Directly
depend on Apollo Server rather than as a peer

### Patch Changes

- Updated dependencies
\[[`2a2d1e3b4`](2a2d1e3),
[`2a2d1e3b4`](2a2d1e3),
[`2a2d1e3b4`](2a2d1e3)]:
    -   @apollo/server@4.1.0

## @apollo/server@4.1.0

### Minor Changes

-
[`2a2d1e3b4`](2a2d1e3)
Thanks [@glasser](https://github.com/glasser)! - The `cache-control`
HTTP response header set by the cache control plugin now properly
reflects the cache policy of all operations in a batched HTTP request.
(If you write the `cache-control` response header via a different
mechanism to a format that the plugin would not produce, the plugin no
longer writes the header.) For more information, see [advisory
GHSA-8r69-3cvp-wxc3](GHSA-8r69-3cvp-wxc3).

-
[`2a2d1e3b4`](2a2d1e3)
Thanks [@glasser](https://github.com/glasser)! - Plugins processing
multiple operations in a batched HTTP request now have a shared
`requestContext.request.http` object. Changes to HTTP response headers
and HTTP status code made by plugins operating on one operation can be
immediately seen by plugins operating on other operations in the same
HTTP request.

-
[`2a2d1e3b4`](2a2d1e3)
Thanks [@glasser](https://github.com/glasser)! - New field
`GraphQLRequestContext.requestIsBatched` available to plugins.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 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.

Figure out how to do a minor version bump
2 participants