fix: Various issues found in testing Ruby client #166
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
meagar
force-pushed
the
ruby-client-tests
branch
4 times, most recently
from
5339787 to
7e8f50e
Compare
| @@ -336,17 +336,18 @@ func TestMutateRows_Retry_ExponentialBackoff(t *testing.T) { | |||
|
|
|||
| // 4b. Log the retry delays | |||
| origReq := <-recorder | |||
| firstRetry := <-recorder | |||
| secondRetry := <-recorder | |||
| thirdRetry := <-recorder | |||
There was a problem hiding this comment.
This code was hanging indefinitely when the client made fewer than three retries.
meagar
force-pushed
the
ruby-client-tests
branch
from
7e8f50e to
102cd3d
Compare
| assert.Nil(t, err, "failed to decode client feature flags") | ||
|
|
||
| assert.True(t, ff.ReverseScans, "client does must enable ReverseScans feature flag") |
There was a problem hiding this comment.
This was segfaulting when ff was nil.
meagar
force-pushed
the
ruby-client-tests
branch
from
102cd3d to
aeafea1
Compare
meagar
force-pushed
the
ruby-client-tests
branch
from
aeafea1 to
78ac1f3
Compare
meagar
force-pushed
the
ruby-client-tests
branch
from
78ac1f3 to
d3e231b
Compare
meagar
changed the title
| // Different clients may have different behaviors, we log the delays for informational purpose. | ||
| // Example: For the first retry delay, C++ client uses 100ms but Java client uses 10ms. | ||
| t.Logf("The three retry delays are: %dms, %dms, %dms", firstDelay, secondDelay, thirdDelay) | ||
| for n := 1; n < numRPCs; n += 1 { |
There was a problem hiding this comment.
Maybe in the end we should assert that n == 3? so it doesn't hang and we can verify the client retried 3 times?
There was a problem hiding this comment.
There's an assertion for this above,
// 4a. Check the number of requests in the recorder
assert.Equal(t, numRPCs, len(recorder))
This lines are not functionally part of the test, they just seem to output debugging information. There are no assertions here.
Co-authored-by: Mattie Fu <mattiefu@google.com>
| // Different clients may have different behaviors, we log the delays for informational purpose. | ||
| // Example: For the first retry delay, C++ client uses 100ms but Java client uses 10ms. | ||
| t.Logf("The three retry delays are: %dms, %dms, %dms", firstDelay, secondDelay, thirdDelay) | ||
| for n := 1; n < numRPCs; n += 1 { |
Introduce a few small fixes to allow the test suite to run against the Ruby test proxy. Several tests were failing either because they waited on a channel that would not receive anything, or they attempted to read through a
nilpointer.The corresponding test proxy is here: googleapis/google-cloud-ruby#25680