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
test: fix numeric compliance test #29
Conversation
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.
There's a lot of skipping relating to decimal which I'm concerned about. Would the changes included in #33 be relevant here?
All the tests passes once googleapis/python-spanner#290 PR merged. |
…n-spanner-sqlalchemy into numeric_compliance_test
…n-spanner-sqlalchemy into numeric_compliance_test
Tests look good to me. LGTM. |
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.
Test changes look good to me. Please wait for Skylar's approval before merging.
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 other than the nit
Failed test will be resolved by PR #62 |
test/test_suite.py
Outdated
of 38 and scale of 9. | ||
""" | ||
self._do_test( | ||
Numeric(precision=18, scale=12), |
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.
We are overriding a number of tests because Spanner does not support scale>9
. But these tests still uses Numeric
with scale=12
or scale=14
. I think it makes sense to change these Numeric
s to use scale=9
. WDYT?
…n-spanner-sqlalchemy into numeric_compliance_test
No description provided.