-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Test result ordering, add --accept test mode to automatically accept new/changed test cases
#2831
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
Conversation
|
This exposes some potential bugs and inconsistencies that still need to be investigated. |
|
Deploy preview for hasura-docs ready! Built with commit 008a3df |
|
Review app for commit c94fbb1 deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
|
Awesome feature |
|
TODO I should strip trailing whitespace from the graphql query string before writing back, so ruamel doesn't do ugly formatting. |
c94fbb1 to
d713c6a
Compare
|
Review app for commit d713c6a deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
|
Review app for commit 8f48bd3 deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
8f48bd3 to
0c7d8e3
Compare
|
Review app for commit 0c7d8e3 deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
0c7d8e3 to
67f74ca
Compare
|
Review app for commit 67f74ca deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
--accept test mode to automatically accept new/changed test cases--accept test mode to automatically accept new/changed test cases
lexi-lambda
left a comment
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 LGTM. Thanks for doing this!
| #!/usr/bin/env python3 | ||
|
|
||
| # This module is for tests that validate our tests or test framework, make sure | ||
| # tests are running correctly, or test our python test helpers. |
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 a nice detail.
|
Review app for commit af547ae deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
|
Cool, thanks! (I don't have permissions to merge to master; let me know if there's anything else you need from me) |
See: https://stackoverflow.com/q/57682657/176841 I don't think this is something we care about, and we need to fix this for ruamel which uses the more sane v1.2 spec.
…t changed test cases We add a new pytest flag `--accept` that will automatically write back yaml files with updated responses. This makes it much easier and less error-prone to update test cases when we expect output to change, or when authoring new tests. Second we make sure to test that we actually preserve the order of the selection set when returning results. This is a "SHOULD" part of the spec but seems pretty important and something that users will rely on. To support both of the above we use ruamel.yaml which preserves a certain amount of formatting and comments (so that --accept can work in a failry ergonomic way), as well as ordering (so that when we write yaml the order of keys has meaning that's preserved during parsing). Use ruamel.yaml everywhere for consistency (since both libraries have different quirks). Quirks of ruamel.yaml: - trailing whitespace in multiline strings in yaml files isn't written back out as we'd like: https://bitbucket.org/ruamel/yaml/issues/47/multiline-strings-being-changed-if-they - formatting is only sort of preserved; ruamel e.g. normalizes indentation. Normally the diff is pretty clean though, and you can always just check in portions of your test file after --accept fixup
These were generated with `--accept` and inspected individually. Mark failing cases as xfail: hasura#3271
af547ae to
008a3df
Compare
|
Review app for commit 008a3df deployed to Heroku: https://hge-ci-pull-2831.herokuapp.com |
|
Review app https://hge-ci-pull-2831.herokuapp.com is deleted |
We add a new pytest flag
--acceptthat will automatically write backyaml files with updated responses. This makes it much easier and less
error-prone to update test cases when we expect output to change, or
when authoring new tests.
Second we make sure to test that we actually preserve the order of the
selection set when returning results. This is a "SHOULD" part of the
spec but seems pretty important and something that users will rely on.
To support both of the above we use ruamel.yaml which preserves a
certain amount of formatting and comments (so that --accept can work in
a failry ergonomic way), as well as ordering (so that when we write yaml
the order of keys has meaning that's preserved during parsing).
Use ruamel.yaml everywhere for consistency (since both libraries have
different quirks).
Quirks of ruamel.yaml:
back out as we'd like: https://bitbucket.org/ruamel/yaml/issues/47/multiline-strings-being-changed-if-they
indentation. Normally the diff is pretty clean though, and you can
always just check in portions of your test file after --accept
UPDATE: marked xfail for the failing case we found case: #3271 . This isn't great since we lose the other functionality those tests were excercising.