Skip to content
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

tests: add get_or_create #385

Merged
merged 2 commits into from
Mar 28, 2020
Merged

tests: add get_or_create #385

merged 2 commits into from
Mar 28, 2020

Conversation

odeke-em
Copy link
Contributor

@odeke-em odeke-em commented Mar 26, 2020

Ran the various component tests in get_or_create,
filed and grouped tests that fail due to lack of savepoints
in Cloud Spanner, following Tim's advisory in #384.

Fixes #270
Fixes #384

@odeke-em odeke-em force-pushed the tests-audit-get_or_create branch 2 times, most recently from e9e2cc9 to d2b639a Compare March 26, 2020 20:39
django_spanner/features.py Outdated Show resolved Hide resolved
django_spanner/features.py Outdated Show resolved Hide resolved
django_spanner/features.py Outdated Show resolved Hide resolved
@timgraham
Copy link
Contributor

As said at #270 (comment), I believe the remaing skips should be grouped with 'test_utils.tests.TestBadSetUpTestData.test_failure_in_setUpTestData_should_rollback_transaction'" under the title "Tests that require savepoints."

Ran the various component tests in get_or_create,
filed and grouped tests that fail due to lack of savepoints
in Cloud Spanner, following Tim's advisory in #384.

Fixes #270
Fixes #384
@odeke-em
Copy link
Contributor Author

As said at #270 (comment), I believe the remaing skips should be grouped with 'test_utils.tests.TestBadSetUpTestData.test_failure_in_setUpTestData_should_rollback_transaction'" under the title "Tests that require savepoints."

Thank you @timgraham! I've added those suggestions and updated this PR with the corresponding issues, and the features.py entries.

Copy link
Contributor

@timgraham timgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all those tests actually failing? (The previous list was shorter, I thought?) I think CockroachDB is a bit different because it supports transactions but not savepoints while Spanner doesn't support transactions nor savepoints.

@odeke-em
Copy link
Contributor Author

Are all those tests actually failing? (The previous list was shorter, I thought?) I think CockroachDB is a bit different because it supports transactions but not savepoints while Spanner doesn't support transactions nor savepoints.

I blindly copied all the mentioned tests in #270 (comment), without any thought. Sure let me re-audit by enabling and running the newly skipped tests, it should take about 12 minutes or so to finish running locally.

@odeke-em
Copy link
Contributor Author

@timgraham results came back

test_get_or_create_invalid_params (get_or_create.tests.GetOrCreateTests) ... ok
test_create_with_duplicate_primary_key (get_or_create.tests.GetOrCreateTestsWithManualPKs) ... ok
test_get_or_create_raises_IntegrityError_plus_traceback (get_or_create.tests.GetOrCreateTestsWithManualPKs) ... ok
test_savepoint_rollback (get_or_create.tests.GetOrCreateTestsWithManualPKs) ... ok
test_something (get_or_create.tests.GetOrCreateThroughManyToMany) ... ok
test_integrity (get_or_create.tests.UpdateOrCreateTests) ... ERROR
test_manual_primary_key_test (get_or_create.tests.UpdateOrCreateTests) ... ERROR
test_create_with_duplicate_primary_key (get_or_create.tests.UpdateOrCreateTestsWithManualPKs) ... ERROR

There were only 3 failing tests so back to the short list, please take a look and thanks again!

django_spanner/features.py Outdated Show resolved Hide resolved
@odeke-em odeke-em merged commit be1ce75 into master Mar 28, 2020
@odeke-em odeke-em deleted the tests-audit-get_or_create branch March 28, 2020 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants