diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-21 14:25:44 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-21 15:00:30 +0300 |
| commit | a5004da3edd386934660e7da96cbcf3b89992ad1 (patch) | |
| tree | 350d7fd824973ca92f08c8995fdd4a4b44782675 | |
| parent | c9a0ae0322d76db096a823fe3d8cee5908250b26 (diff) | |
commit: Fix test timeout compared by inefficient go-cmp diffing
Commit signature tests are frequently timing out in our CI, where a
single testcase frequently takes more than 400 seconds to succeed. Quite
surprisingly, the root cause is that we're using go-cmp to diff received
Proto messages. The diff algorithm used by go-cmp seems to be
inefficient enough to choke when comparing these messages, which contain
multiple megabytes of signed data.
Fix the issue by comparing the proto message's fields for equality
directly. On my machine, this decreases the runtime of this testcase
from more than 16 seconds to a fraction of a second. While this is not
the same numbers as in our CI, my machine is probably a lot more beefy.
| -rw-r--r-- | internal/gitaly/service/commit/commit_signatures_test.go | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/internal/gitaly/service/commit/commit_signatures_test.go b/internal/gitaly/service/commit/commit_signatures_test.go index b8d3fd455..da2f73587 100644 --- a/internal/gitaly/service/commit/commit_signatures_test.go +++ b/internal/gitaly/service/commit/commit_signatures_test.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v14/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" - "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert" "gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb" "google.golang.org/grpc/codes" ) @@ -68,7 +67,12 @@ func TestSuccessfulGetCommitSignaturesRequest(t *testing.T) { require.Len(t, fetchedSignatures, len(expectedSignatures)) for i, expected := range expectedSignatures { - testassert.ProtoEqual(t, expected, fetchedSignatures[i]) + // We cannot use `testassert.ProtoEqual` here due to it being too inefficient with + // the data we're comparing because it contains multiple MB of signed data. This has + // in the past led to frequent timeouts in CI. + require.Equal(t, expected.CommitId, fetchedSignatures[i].CommitId) + require.Equal(t, expected.Signature, fetchedSignatures[i].Signature) + require.Equal(t, expected.SignedText, fetchedSignatures[i].SignedText) } } |