diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-24 16:13:46 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-30 09:09:53 +0300 |
commit | 86b607c1ffb7a7457ad567a68637603e09333c38 (patch) | |
tree | efcd5b5157cd7d22adc4ad886e2eeb8c5c189352 | |
parent | fb40241b793fb700cf1d0604199b320b296ec548 (diff) |
gitaly-lfs-smudge: Do not test logging in unit tests
We're currently verifying that some of our low-level functions log
certain events into the logfile. This makes it hard to refactor the
code, and ultimately we really should only exercise the system under
test instead of also verifying that some other components like logging
work as expected. As another hurdle, this is effectively testing global
state as the loggers are configured globally.
Remove these assertions from the lower-level unit tests to allow for
easier refactoring of the code. Logging is tested already via our new
high-level tests which verify that executing the binary does the right
thing.
-rw-r--r-- | cmd/gitaly-lfs-smudge/lfs_smudge_test.go | 73 |
1 files changed, 18 insertions, 55 deletions
diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go index 32f8e254e..3bbfaf39b 100644 --- a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go +++ b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go @@ -4,14 +4,12 @@ import ( "bytes" "encoding/json" "net/http" - "path/filepath" "strings" "testing" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitlab" - "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" ) const ( @@ -86,46 +84,29 @@ func TestSuccessfulLfsSmudge(t *testing.T) { }) require.NoError(t, err) - tmpDir := testhelper.TempDir(t) - env := map[string]string{ "GL_REPOSITORY": "project-1", "GL_INTERNAL_CONFIG": string(cfg), - "GITALY_LOG_DIR": tmpDir, "GITALY_TLS": string(tlsCfg), } cfgProvider := &mapConfig{env: env} - _, err = initLogging(cfgProvider) - require.NoError(t, err) err = smudge(&b, reader, cfgProvider) require.NoError(t, err) require.Equal(t, testData, b.String()) - - logFilename := filepath.Join(tmpDir, "gitaly_lfs_smudge.log") - require.FileExists(t, logFilename) - - data := testhelper.MustReadFile(t, logFilename) - require.NoError(t, err) - d := string(data) - - require.Contains(t, d, `"msg":"Finished HTTP request"`) - require.Contains(t, d, `"status":200`) - require.Contains(t, d, `"content_length_bytes":`) }) } } func TestUnsuccessfulLfsSmudge(t *testing.T) { testCases := []struct { - desc string - data string - missingEnv string - tlsCfg config.TLS - expectedError bool - options gitlab.TestServerOptions - expectedLogMessage string - expectedGitalyTLS string + desc string + data string + missingEnv string + tlsCfg config.TLS + expectedError bool + options gitlab.TestServerOptions + expectedGitalyTLS string }{ { desc: "bad LFS pointer", @@ -146,20 +127,18 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) { expectedError: false, }, { - desc: "missing GL_REPOSITORY", - data: lfsPointer, - missingEnv: "GL_REPOSITORY", - options: defaultOptions, - expectedError: true, - expectedLogMessage: "GL_REPOSITORY is not defined", + desc: "missing GL_REPOSITORY", + data: lfsPointer, + missingEnv: "GL_REPOSITORY", + options: defaultOptions, + expectedError: true, }, { - desc: "missing GL_INTERNAL_CONFIG", - data: lfsPointer, - missingEnv: "GL_INTERNAL_CONFIG", - options: defaultOptions, - expectedError: true, - expectedLogMessage: "unable to retrieve GL_INTERNAL_CONFIG", + desc: "missing GL_INTERNAL_CONFIG", + data: lfsPointer, + missingEnv: "GL_INTERNAL_CONFIG", + options: defaultOptions, + expectedError: true, }, { desc: "failed HTTP response", @@ -171,8 +150,7 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) { GlRepository: glRepository, LfsStatusCode: http.StatusInternalServerError, }, - expectedError: true, - expectedLogMessage: "error loading LFS object", + expectedError: true, }, { desc: "invalid TLS paths", @@ -194,12 +172,9 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) { tlsCfg, err := json.Marshal(tc.tlsCfg) require.NoError(t, err) - tmpDir := testhelper.TempDir(t) - env := map[string]string{ "GL_REPOSITORY": "project-1", "GL_INTERNAL_CONFIG": string(cfg), - "GITALY_LOG_DIR": tmpDir, "GITALY_TLS": string(tlsCfg), } @@ -212,9 +187,6 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) { var b bytes.Buffer reader := strings.NewReader(tc.data) - _, err = initLogging(cfgProvider) - require.NoError(t, err) - err = smudge(&b, reader, cfgProvider) if tc.expectedError { @@ -223,15 +195,6 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) { require.NoError(t, err) require.Equal(t, tc.data, b.String()) } - - logFilename := filepath.Join(tmpDir, "gitaly_lfs_smudge.log") - require.FileExists(t, logFilename) - - data := testhelper.MustReadFile(t, logFilename) - - if tc.expectedLogMessage != "" { - require.Contains(t, string(data), tc.expectedLogMessage) - } }) } } |