Skip to content

test: second batch of integration tests#113

Merged
prawilny merged 1 commit intoac/integration-tests-2.x/1from
ac/integration-tests-2.x/2
Nov 1, 2021
Merged

test: second batch of integration tests#113
prawilny merged 1 commit intoac/integration-tests-2.x/1from
ac/integration-tests-2.x/2

Conversation

@prawilny
Copy link
Copy Markdown
Collaborator

@prawilny prawilny commented Oct 23, 2021

Status of 1.x integration tests:

  • TestMirroringTable - done with exception of all kinds of checkAndMutate() [after an hour of debugging I decided to ask for help]
  • TestBlocking - done (one test runs for too long in BigtableToHBase2 configuration)
  • TestErrorDetection - done (one test erred in BigtableToHBase2 configuration)
  • TestBufferedMutator - feature not implemented
  • TestReadVerificationSampling - feature not implemented

This change is Reviewable

@mwalkiewicz
Copy link
Copy Markdown

mwalkiewicz commented Oct 25, 2021

Those tests are mostly copied from 1.x tests, can't we somehow generalize 1.x tests and use the generalized version here? It won't be straightforward because we cannot make them generic and pass either Async or non-Async connection, because they do not have a common base class. However we can create one - lets say we create a TestConnectionCompat interface. It has all the method that are used in tests (get, put, getScanner, etc.), we make two implementations, one in 1.x which has a MirroringConnection as a member and forwards calls to this member, and 2.x has a MirroringAsyncConnection which implements get() as get(...).get(), etc. The implementation would be specified using a property and ConnectionRule could return this wrapper. This way you'd be able to reuse all the code, and explicitly test only those cases where the behavior is 2.x specific. It seems that Blocking and ErrorDetection can be reused in this way, WDYT? (We have to decide is this is a 2h rewrite or full day operation before implementing this change).

I can also see that you are sometimes waiting for the futures to complete and sometimes you are not, is it intended?

@mwalkiewicz mwalkiewicz linked an issue Oct 25, 2021 that may be closed by this pull request
@prawilny
Copy link
Copy Markdown
Collaborator Author

As discussed F2F I ignored the comment above.

Fixed checkAndMutate tests. Found a bug in reference counting (ListenableReferenceCounter's counter reaches negative numbers) in MirroringAsyncTable (in wrapWithReferenceCounter most probably).

@mwalkiewicz
Copy link
Copy Markdown

Can you merge this PR with first batch and rename it to "test: 2.x integration tests"?

@prawilny prawilny force-pushed the ac/integration-tests-2.x/1 branch 2 times, most recently from cbafc05 to 3e19c19 Compare November 1, 2021 14:35
@prawilny prawilny force-pushed the ac/integration-tests-2.x/2 branch from 921e862 to d6b9ffe Compare November 1, 2021 14:54
@prawilny prawilny marked this pull request as ready for review November 1, 2021 14:55
@prawilny prawilny merged commit 97c5a4f into ac/integration-tests-2.x/1 Nov 1, 2021
@prawilny prawilny deleted the ac/integration-tests-2.x/2 branch November 2, 2021 14:00
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

Successfully merging this pull request may close these issues.

2.x: client integration tests

2 participants