tree 80febc918a41f3d59fa19d9ef0c3d8d93b2363b1
parent edf9523a34b344928c2061c06509556636e52cb3
author Jean de Klerk <deklerk@google.com> 1544629737 -0800
committer Jean de Klerk <deklerk@google.com> 1545094784 +0000

spanner: refactor mockclient

This CL refactors MockCloudSpannerClient, removing the Actions
expectations/return value setup slice, the nice bool (fail all requests), and
the injErr map (fail specific request). These are replaced by a simpler
ReceivedRequests channel that users can assert on to introspect requests sent
to the stub, as well as a simple function-injectable wrapper to perform
test-specific overloading of methods for things like errors.

This CL is in preparation for a future unit test that asserts on the contents
of a request (which we can now introspect using ReceivedRequests). This CL
also attempts to make these tests more readable, by moving assertions and
custom response logic directly into the test instead of having to figure
out how different methods in the mock behave.

This CL also does a large amount of general clean-up:

- setup and mockClient merged into a single function, serverClientMock, that
inits the client and the mock. Also, consolidate all test cleanup into a
callback, and make sure the callback gets called everywhere.
- TestReadOnlyAcquire split into several tests.
- Amazingly integration_test.go's TestMain was causing -short to skip ALL
tests. Its preconditions have been split into a new function
initIntegrationTest to prevent this, causing -short to once again work as
expected.
- prepare renamed prepareIntegrationTest.
- All integration tests renamed to TestIntegration_*.
- Moved TestStructParametersBind to integration_test.go, renamed
TestIntegration_StructParametersBind.
- Moved test-specific variables like errAbrt from global scope to the one
test that uses them.
- Call and assign context.Background once instead of re-calling it everywhere
that context is expected.
- Convert to using Fatalf instead of Errorf. This is slightly overkill: Errorf
should be preferred. However, these tests tend to snowball fail such that the
first failure causes all subsequent assertions to fail. In the future we might
want to revisit this and look more closely.
- Re-write several test descriptions to be understandable and clear.
- Re-write several test names to be more indicative of what's being tested.
- Re-write several test assertions to use better failure descriptions, as well
as %+v.
- Removed several testing.Short->t.Skip statements from unit tests (should
only be on integration tests and the like).

Change-Id: I100319acad62db08da1f168bf5087bdfccd3b6b2
Reviewed-on: https://code-review.googlesource.com/c/36270
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Eno Compton <enocom@google.com>