diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-19 10:18:21 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-19 10:57:14 +0300 |
commit | 1fba2d9e14873c8061c4c56d525525553b7cf71f (patch) | |
tree | 4d300cdf0a30cdf9d77c8755201a10d2959b821c | |
parent | 2106629e3af3e8949b23f20825d6bfee62c10992 (diff) |
operations: Fix flaky test for known-to-fail requests in UserApplyPatch
We have observed that one of our tests for `UserApplyPatch` is flaky:
from time to time we receive an unexpected EOF while streaming the patch
data to the server side. As it turns out, this flakiness is our own
fault: we send a request header that is known to cause failure, which
means that the server will return an error as soon at it realizes that.
This is effectively causing a race between the client, which is trying
to send the patch data to the server, and the server which is realizing
the error and thus aborting the RPC call.
Fix this flake by stopping on EOF so that we can test for the real error
condition via `CloseAndRecv()`.
-rw-r--r-- | internal/gitaly/service/operations/apply_patch_test.go | 20 |
1 files changed, 18 insertions, 2 deletions
diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go index c7e5ffb15..9cfffa603 100644 --- a/internal/gitaly/service/operations/apply_patch_test.go +++ b/internal/gitaly/service/operations/apply_patch_test.go @@ -373,14 +373,30 @@ To restore the original branch and stop patching, run "git am --abort". }, })) + outerLoop: for _, patch := range patches { // we stream the patches one rune at a time to exercise the streaming code for _, r := range patch { - require.NoError(t, stream.Send(&gitalypb.UserApplyPatchRequest{ + err := stream.Send(&gitalypb.UserApplyPatchRequest{ UserApplyPatchRequestPayload: &gitalypb.UserApplyPatchRequest_Patches{ Patches: []byte{r}, }, - })) + }) + + // In case the request we're sending to the server results + // in an error it can happen that the server already noticed + // the request and thus returned an error while we're still + // streaming the actual patch data. If so, the server closes + // the stream and we are left unable to continue sending, + // which means we get an EOF here. + // + // Abort the loop here so that we can observe the actual + // error in `CloseAndRecv()`. + if err == io.EOF { + break outerLoop + } + + require.NoError(t, err) } } |