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

chore: add tests #7

Merged
merged 4 commits into from
Dec 4, 2019
Merged

chore: add tests #7

merged 4 commits into from
Dec 4, 2019

Conversation

phryneas
Copy link
Contributor

This adds

  • snapshot tests for the extended schema
  • queries against schema & db against _entites with
    • nodeId
    • a single primary key
    • different types via single primary key
    • a combined primary key
  • an integration test that starts postgraphile, a federated service and an apollo gateway and
    • snapshot-tests the generated schema
    • fires a query and validates that an additional property is added by the federated service

As this PR includes #3 (it wouldn't make sense to me to not test that while I'm at it), the diff is a bit messy.
You can get a cleaner diff here:
https://github.com/phryneas/federation/compare/pr-3...phryneas:pr-5?expand=1

@phryneas
Copy link
Contributor Author

And it's running in Travis. Gonna head home :)

Oh, once more: this was done in @mayflower slack time. If there's gonna be any Release Notes I'd be happy for a mention :)

@benjie
Copy link
Member

benjie commented Nov 19, 2019

I'd like to see this split into two PRs: one that only adds tests for the existing functionality (so that we can be sure that nothing has been broken); and then a second PR that adds the code changes included (and relevant additional tests).

@phryneas
Copy link
Contributor Author

Sure, I'll do that within the next few days.

Lenz Weber and others added 4 commits November 23, 2019 15:48
(cherry picked from commit 7284764)
(cherry picked from commit 3197496)
(cherry picked from commit 85a252e)
@phryneas
Copy link
Contributor Author

@benjie okay, tests for #3 are removed, this now just tests the current master.
I'll add those back to #3 once this is merged :)

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@benjie benjie changed the title add tests chore: add tests Dec 4, 2019
@benjie benjie merged commit e4dca9c into graphile:master Dec 4, 2019
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.

2 participants