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

Create Database Integration Test Suite #1740

Closed
gdbelvin opened this issue Jun 27, 2019 · 7 comments
Closed

Create Database Integration Test Suite #1740

gdbelvin opened this issue Jun 27, 2019 · 7 comments

Comments

@gdbelvin
Copy link
Contributor

Each database implementation has its own set of "unit tests"
We should really be testing these implementations at the interface layer so we can have some confidence that the assumptions we have about one storage implementation carry to the other implementations

@gdbelvin
Copy link
Contributor Author

gdbelvin commented Jul 9, 2019

The storage integration test has revealed an interesting edge case.

Here's the cloud spanner documentation:

Commit might return an ABORTED error. This can occur at any time; commonly, the cause is conflicts with concurrent transactions. However, it can also happen for a variety of other reasons. If Commit returns ABORTED, the caller should re-attempt the transaction from the beginning, re-using the same session.

There are a new instances of

	err := admin.ReadWriteTransaction(ctx, func(ctx context.Context, tx AdminTX) error {

That do not retry on ABORTED.
These should probably be moved to RunInAdminSnapshot or RunInTransaction.
RunInX should also be modified to retry on ABORTED errors.

@gdbelvin
Copy link
Contributor Author

gdbelvin commented Jul 9, 2019

@AlCutter for cloud spanner API comments

@gdbelvin gdbelvin changed the title Create Database Integration Test Harness Create Database Integration Test Suite Jul 9, 2019
@gdbelvin
Copy link
Contributor Author

Implementation ongoing:
#2134
#2133
#2132

@gdbelvin
Copy link
Contributor Author

Working cloudspanner :-D
cc: @AlCutter @rmhrisk

@rmhrisk
Copy link

rmhrisk commented Jun 29, 2020

Woot woot ;)

@gdbelvin gdbelvin mentioned this issue Jun 30, 2020
2 tasks
@gdbelvin gdbelvin assigned pav-kv and unassigned gdbelvin Jul 2, 2020
@pav-kv pav-kv removed their assignment Mar 14, 2022
@mhutchinson
Copy link
Contributor

@pavelkalinnikov do you have any context on what work is remaining here?

@pav-kv
Copy link
Contributor

pav-kv commented May 27, 2022

integration/storagetest contains generic tests that can be used to test any storage implementation. If I remember correctly, it works for the storages in this repo, and what was left is making it work for our internal storage implementation. We can track it internally though.

@pav-kv pav-kv closed this as completed May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants