summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2020-05-08 15:40:18 -0700
committerStan Hu <stanhu@gmail.com>2020-05-08 15:40:18 -0700
commitc0cf314c5722a4f7ec0a1f355d23fa2511344989 (patch)
tree96898bec50690d0d34eb9ead815b8ef4a89a40ce
parent1adadc2e299529e805aae3dac7c2579b88c8dae7 (diff)
downloadgitlab-shell-sh-add-http-status-code.tar.gz
Fix race conditions in testssh-add-http-status-code
Calling logrus hook.LastEntry() can lead to race conditions. Use AllEntries instead: https://github.com/sirupsen/logrus/blob/60c74ad9be0d874af0ab0daef6ab07c5c5911f0d/hooks/test/test.go#L77 Closes https://gitlab.com/gitlab-org/gitlab-shell/-/issues/450
-rw-r--r--client/client_test.go66
-rw-r--r--internal/command/receivepack/gitalycall_test.go10
-rw-r--r--internal/command/uploadarchive/gitalycall_test.go9
-rw-r--r--internal/command/uploadpack/gitalycall_test.go10
4 files changed, 51 insertions, 44 deletions
diff --git a/client/client_test.go b/client/client_test.go
index 6c4268e..5e852dc 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -123,11 +123,12 @@ func testSuccessfulGet(t *testing.T, client *GitlabNetClient) {
assert.NoError(t, err)
assert.Equal(t, string(responseBody), "Hello")
- assert.Equal(t, 1, len(hook.Entries))
- assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- assert.Contains(t, hook.LastEntry().Message, "method=GET")
- assert.Contains(t, hook.LastEntry().Message, "status=200")
- assert.Contains(t, hook.LastEntry().Message, "Finished HTTP request")
+ entries := hook.AllEntries()
+ assert.Equal(t, 1, len(entries))
+ assert.Equal(t, logrus.InfoLevel, entries[0].Level)
+ assert.Contains(t, entries[0].Message, "method=GET")
+ assert.Contains(t, entries[0].Message, "status=200")
+ assert.Contains(t, entries[0].Message, "Finished HTTP request")
})
}
@@ -146,11 +147,12 @@ func testSuccessfulPost(t *testing.T, client *GitlabNetClient) {
assert.NoError(t, err)
assert.Equal(t, "Echo: {\"key\":\"value\"}", string(responseBody))
- assert.Equal(t, 1, len(hook.Entries))
- assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- assert.Contains(t, hook.LastEntry().Message, "method=POST")
- assert.Contains(t, hook.LastEntry().Message, "status=200")
- assert.Contains(t, hook.LastEntry().Message, "Finished HTTP request")
+ entries := hook.AllEntries()
+ assert.Equal(t, 1, len(entries))
+ assert.Equal(t, logrus.InfoLevel, entries[0].Level)
+ assert.Contains(t, entries[0].Message, "method=POST")
+ assert.Contains(t, entries[0].Message, "status=200")
+ assert.Contains(t, entries[0].Message, "Finished HTTP request")
})
}
@@ -161,11 +163,12 @@ func testMissing(t *testing.T, client *GitlabNetClient) {
assert.EqualError(t, err, "Internal API error (404)")
assert.Nil(t, response)
- assert.Equal(t, 1, len(hook.Entries))
- assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- assert.Contains(t, hook.LastEntry().Message, "method=GET")
- assert.Contains(t, hook.LastEntry().Message, "status=404")
- assert.Contains(t, hook.LastEntry().Message, "Internal API error")
+ entries := hook.AllEntries()
+ assert.Equal(t, 1, len(entries))
+ assert.Equal(t, logrus.InfoLevel, entries[0].Level)
+ assert.Contains(t, entries[0].Message, "method=GET")
+ assert.Contains(t, entries[0].Message, "status=404")
+ assert.Contains(t, entries[0].Message, "Internal API error")
})
t.Run("Missing error for POST", func(t *testing.T) {
@@ -174,11 +177,12 @@ func testMissing(t *testing.T, client *GitlabNetClient) {
assert.EqualError(t, err, "Internal API error (404)")
assert.Nil(t, response)
- assert.Equal(t, 1, len(hook.Entries))
- assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- assert.Contains(t, hook.LastEntry().Message, "method=POST")
- assert.Contains(t, hook.LastEntry().Message, "status=404")
- assert.Contains(t, hook.LastEntry().Message, "Internal API error")
+ entries := hook.AllEntries()
+ assert.Equal(t, 1, len(entries))
+ assert.Equal(t, logrus.InfoLevel, entries[0].Level)
+ assert.Contains(t, entries[0].Message, "method=POST")
+ assert.Contains(t, entries[0].Message, "status=404")
+ assert.Contains(t, entries[0].Message, "Internal API error")
})
}
@@ -204,11 +208,12 @@ func testBrokenRequest(t *testing.T, client *GitlabNetClient) {
assert.EqualError(t, err, "Internal API unreachable")
assert.Nil(t, response)
- assert.Equal(t, 1, len(hook.Entries))
- assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- assert.Contains(t, hook.LastEntry().Message, "method=GET")
- assert.NotContains(t, hook.LastEntry().Message, "status=")
- assert.Contains(t, hook.LastEntry().Message, "Internal API unreachable")
+ entries := hook.AllEntries()
+ assert.Equal(t, 1, len(entries))
+ assert.Equal(t, logrus.InfoLevel, entries[0].Level)
+ assert.Contains(t, entries[0].Message, "method=GET")
+ assert.NotContains(t, entries[0].Message, "status=")
+ assert.Contains(t, entries[0].Message, "Internal API unreachable")
})
t.Run("Broken request for POST", func(t *testing.T) {
@@ -218,11 +223,12 @@ func testBrokenRequest(t *testing.T, client *GitlabNetClient) {
assert.EqualError(t, err, "Internal API unreachable")
assert.Nil(t, response)
- assert.Equal(t, 1, len(hook.Entries))
- assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- assert.Contains(t, hook.LastEntry().Message, "method=POST")
- assert.NotContains(t, hook.LastEntry().Message, "status=")
- assert.Contains(t, hook.LastEntry().Message, "Internal API unreachable")
+ entries := hook.AllEntries()
+ assert.Equal(t, 1, len(entries))
+ assert.Equal(t, logrus.InfoLevel, entries[0].Level)
+ assert.Contains(t, entries[0].Message, "method=POST")
+ assert.NotContains(t, entries[0].Message, "status=")
+ assert.Contains(t, entries[0].Message, "Internal API unreachable")
})
}
diff --git a/internal/command/receivepack/gitalycall_test.go b/internal/command/receivepack/gitalycall_test.go
index dd75176..fd062b2 100644
--- a/internal/command/receivepack/gitalycall_test.go
+++ b/internal/command/receivepack/gitalycall_test.go
@@ -2,7 +2,6 @@ package receivepack
import (
"bytes"
- "strings"
"testing"
"github.com/sirupsen/logrus"
@@ -43,7 +42,10 @@ func TestReceivePack(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "ReceivePack: "+userId+" "+repo, output.String())
- require.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- require.True(t, strings.Contains(hook.LastEntry().Message, "executing git command"))
- require.True(t, strings.Contains(hook.LastEntry().Message, "command=git-receive-pack"))
+
+ entries := hook.AllEntries()
+ require.Equal(t, 2, len(entries))
+ require.Equal(t, logrus.InfoLevel, entries[1].Level)
+ require.Contains(t, entries[1].Message, "executing git command")
+ require.Contains(t, entries[1].Message, "command=git-receive-pack")
}
diff --git a/internal/command/uploadarchive/gitalycall_test.go b/internal/command/uploadarchive/gitalycall_test.go
index 95274ee..8b60e11 100644
--- a/internal/command/uploadarchive/gitalycall_test.go
+++ b/internal/command/uploadarchive/gitalycall_test.go
@@ -2,7 +2,6 @@ package uploadarchive
import (
"bytes"
- "strings"
"testing"
"github.com/sirupsen/logrus"
@@ -43,7 +42,9 @@ func TestUploadPack(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "UploadArchive: "+repo, output.String())
- require.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- require.True(t, strings.Contains(hook.LastEntry().Message, "executing git command"))
- require.True(t, strings.Contains(hook.LastEntry().Message, "command=git-upload-archive"))
+ entries := hook.AllEntries()
+ require.Equal(t, 2, len(entries))
+ require.Equal(t, logrus.InfoLevel, entries[1].Level)
+ require.Contains(t, entries[1].Message, "executing git command")
+ require.Contains(t, entries[1].Message, "command=git-upload-archive")
}
diff --git a/internal/command/uploadpack/gitalycall_test.go b/internal/command/uploadpack/gitalycall_test.go
index 4421d41..cf3e621 100644
--- a/internal/command/uploadpack/gitalycall_test.go
+++ b/internal/command/uploadpack/gitalycall_test.go
@@ -2,11 +2,8 @@ package uploadpack
import (
"bytes"
- "strings"
"testing"
- "github.com/sirupsen/logrus"
-
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -44,9 +41,10 @@ func TestUploadPack(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "UploadPack: "+repo, output.String())
- require.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
- require.True(t, strings.Contains(hook.LastEntry().Message, "executing git command"))
- require.True(t, strings.Contains(hook.LastEntry().Message, "command=git-upload-pack"))
+ entries := hook.AllEntries()
+ assert.Equal(t, 2, len(entries))
+ require.Contains(t, entries[1].Message, "executing git command")
+ require.Contains(t, entries[1].Message, "command=git-upload-pack")
for k, v := range map[string]string{
"gitaly-feature-cache_invalidator": "true",