-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
server/tests: port pytests to hspec integration test suite #8432
Comments
…ror in CI Fixes hasura/graphql-engine-mono#3695. Error: [BigQuery test failure Job exceeded rate limits](hasura/graphql-engine-mono#3695) Cause: 1. [this command](https://github.com/hasura/graphql-engine/blob/2325755954bb3a777403503d709b412e01219ba9/.circleci/test-server.sh#L1263) runs tests matching the `Bigquery or Common` string, for the `test-oss-server-bigquery` CI job. 2. in this case, the pytest filter matched on `TestGraphQLQueryBoolExpSearchCommon`. Although unrelated pytests are skipped, BQ setup and teardown runs uneccesarily for the [MSSQL and Postgres backends](https://github.com/hasura/graphql-engine/blob/e444cf1f5d5eb1762357266d8b298b1dfb48d937/server/tests-py/test_graphql_queries.py#L868). 4. the setup and teardown runs three times in quick succession, _for each of_ SQL Server, Postgres and BigQuery. Occasionally, this surpassed [BigQuery's maximum rate of 5 table update operations in 10 seconds](https://cloud.google.com/bigquery/quotas#load_job_per_table.long). Fix: restrict setup/teardown to only the relevant backends... - Hotfix (this PR): ...by renaming pytest classes and changing the pytest filters in `test-server` - ok, this is faintly horrifying and an inelegant convention change. On the bright side, it shaves a minute or so off our integration test suite run by skipping fewer tests. Anecdata for `test-oss-server-bigquery` - before: 87 passed, 299 skipped, 1 warning, 1 error in 192.99s - after: 87 passed, 20 skipped, 1 warning in 170.82s - [`Common` was a terrible name, anyway](hasura/graphql-engine-mono#2079), for `AnyCombinationOfBackends`. - Better fix: ...by refactoring the `conftest.py` helpers. I ran out of a timebox so will write up a separate issue. Given we're actively [porting pytests over to hspec](#8432), I don't know how much it's worth investing time in a refactor. To verify the fix: I ran a full CI build a few times [[1]](https://buildkite.com/hasura/graphql-engine-mono/builds/8069#078c781a-c8ef-44f2-a400-15f91fb88e42)[[2]](https://buildkite.com/hasura/graphql-engine-mono/builds/8072#f9e7f59d-264f-46a4-973d-21aa762cca35)[[3]](https://buildkite.com/hasura/graphql-engine-mono/builds/8075#bb104e80-ff76-408c-a46b-6f40e92e6317) whilst troubleshooting to convince myself this fixed the problem. PR-URL: hasura/graphql-engine-mono#4362 GitOrigin-RevId: 4c3283f0654b70e9dcda642d9012f6376aa95290
These tests also seem to fail for unrelated reasons quite often in CI, making things slower. I wonder how much effort is worth to put into improving the failure rate, versus migrating off the test suite entirely. |
Do you have any recent examples? If so, mind adding them as comments here? Gil did a bunch of work to stabilise the BQ tests in CI, but I wasn't aware of other recent failures.
Good question. I can't speak with any confidence about this until we measure the success rate in CI 😄 otherwise we're just working with anecdata. I created an issue to discuss at backlog refinement, so we can improve visibility here. |
Closing this issue in favour of internal JIRA tickets |
Relates to #7760
This is a placeholder for now, for related tests as we think of them. But we might use this to coordinate work to gradually port pytests to hspecs, now that the POC integration test suite is done. In rough priority order:
server/tests: enable cockroachdb in the pytest suite #8891The text was updated successfully, but these errors were encountered: