-
Notifications
You must be signed in to change notification settings - Fork 138
chore: Make it practical to make simultaneous compliance test runs. #166
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The source of the dependency bug is in old versions of google-cloud-core that depend on too-old versions of google-api-core.
Some tests are still failing, but we're far enough along that we have the right shape, I think.
Other fixes: - Handle BIGINT - Fix string leteral formatting (and start type-specific adaptations).
The SQLAlchemy like convenience functions (e.g. ) escape incorrectly for BigQuery, so re-escape.
We could make that work, if we want to. :)
…ry keys, etc., that BigQuery doesn't have.
The source of the dependency bug is in old versions of google-cloud-core that depend on too-old versions of google-api-core.
So we don't have t mock at the api level.
- Run tests in temporary directory rather than sharing memory connections. Because simpler. :) - Introduce cross-connection state and record queries in it, so tests can make assertions bout generated queries.
…laceholder The BigQuery Python Client supports an extended placeholder syntax that includes type information, as in `%(foo:INT64)s` (named) or `%(:INT64)s` (unnamed). When we know the type, include it in the placeholder.
The numeric tests now tun since we started passing type info from sqla to bigquery. The CTE tests test features that don't exist in bigquery.
Although the test isn't actually testing dialect code. Maybe it should be skipped instead. Also set the profile test pasth to a more reasonable value, although it doesn't seem to be used. <shrug>
Also inlined colspecs code, because there wasn't much and it facilitated separating literal processing into a function.
It didn't buy us anything.
Because: - It duplicated the `--dropfirst` option. - Unlike `--dropfirst`, it didn't honor the `--dburi` option that lets you use an alternate URL/dataset.
The schema-tests schema is hard coded, and cleaning it and running it's tests could conflict with other test runs.
Also, use multiple simple --only-rerun options rather than one complex one.
plamut
approved these changes
May 13, 2021
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.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
api: bigquery
Issues related to the googleapis/python-bigquery-sqlalchemy API.
cla: yes
This human has signed the Contributor License Agreement.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #165 🦕
This changes SQLAlchemy-provided compliance tests to run in temporary datasets, so multiple test runs can happen simultaneously.
Unfortunately, this requires disabling the tests that verify that you can refer to tables with dataset/schema names
mydataset.mytable
be disabled by default, as the schema used by these tests is hard-coded. These tests can still be run manually using--requirements pybigquery.requirements:WithSchemas
. The system tests exercise this functionality.BTW, the system tests need the same treatment. That will be in a separate PR.