make output deterministic for graphql interfaces

Khan / genqlient

a truly type-safe Go GraphQL client

MIT License

1.07k stars 107 forks source link

make output deterministic for graphql interfaces #209

Closed vikstrous2 closed 2 years ago

vikstrous2 commented 2 years ago

We noticed that the output from genqlient can be non-deterministic when a graphql query queries a field that's an interface. This PR fixes that by sorting the types as soon as we extract them from the schema.

The snapshots for the tests need to be updated, but I don't know an easy way to do that. Is there a command or a flag that I can just flip somewhere to update them?

I have:

[x] Written a clear PR title and description (above)
[x] Signed the Khan Academy CLA
[ ] Added tests covering my changes, if applicable
n/a Included a link to the issue fixed, if applicable
n/a Included documentation, for new features
[x] Added an entry to the changelog

StevenACoffman commented 2 years ago

@vikstrous2 Looks like some test failures

benjaminjkraft commented 2 years ago

Huh, I'm a bit surprised since we've never run into this in the tests (or at Khan as far as I know). I'm guessing the issue is actually if you have multiple schema files and the order they get read in is nondeterministic, or something. But anyway, it makes good sense to make this more stable even if in principle we could solve it elsewhere, so I'm happy to merge once tests are updated.

For the tests, it should just be UPDATE_SNAPSHOTS=1 make check.

StevenACoffman commented 2 years ago

Yeah, I ran UPDATE_SNAPSHOTS=1 make check and then pushed the changed version to this PR to clarify which test failures were not fixed by that.

benjaminjkraft commented 2 years ago

Ah! It looks like we also just need go generate ./... to update the genqlient-generated files for integration tests.

benjaminjkraft commented 2 years ago

Ok, there we go!