Drop tests that check for log messages

logrus starts a Goroutine to write messages to the output buffer, but
there is no way to flush these messages to ensure they are ready
(https://github.com/sirupsen/logrus/issues/435). To avoid race
conditions, drop these log checks from our tests.

changelogs/unreleased/sh-remove-log-tests.yml

+---
+title: Drop tests that check for log messages
+merge_request: 591
+author:
+type: other

internal/filestore/file_handler_test.go

@@ -203,11 +203,8 @@ func TestSaveFile(t *testing.T) {
 
 	for _, spec := range tests {
 		t.Run(spec.name, func(t *testing.T) {
-			logHook := testhelper.SetupLogger()
-
 			var opts filestore.SaveFileOpts
 			var expectedDeletes, expectedPuts int
-			var expectedClientMode string
 
 			osStub, ts := test.StartObjectStore()
 			defer ts.Close()
@@ -224,7 +221,6 @@ func TestSaveFile(t *testing.T) {
 
 				expectedDeletes = 1
 				expectedPuts = 1
-				expectedClientMode = "http"
 			case remoteMultipart:
 				objectURL := ts.URL + test.ObjectPath
 
@@ -239,13 +235,11 @@ func TestSaveFile(t *testing.T) {
 				osStub.InitiateMultipartUpload(test.ObjectPath)
 				expectedDeletes = 1
 				expectedPuts = 2
-				expectedClientMode = "multipart"
 			}
 
 			if spec.local {
 				opts.LocalTempPath = tmpFolder
 				opts.TempFilePrefix = "test-file"
-				expectedClientMode = "local"
 			}
 
 			ctx, cancel := context.WithCancel(context.Background())
@@ -295,20 +289,11 @@ func TestSaveFile(t *testing.T) {
 			uploadFields := token.Claims.(*testhelper.UploadClaims).Upload
 
 			checkFileHandlerWithFields(t, fh, uploadFields, "", spec.remote == notRemote)
-
-			require.True(t, testhelper.WaitForLogEvent(logHook))
-			entries := logHook.AllEntries()
-			require.Equal(t, 1, len(entries))
-			msg := entries[0].Message
-			require.Contains(t, msg, "saved file")
-			require.Contains(t, msg, fmt.Sprintf("client_mode=%s", expectedClientMode))
 		})
 	}
 }
 
 func TestSaveFileWithS3WorkhorseClient(t *testing.T) {
-	logHook := testhelper.SetupLogger()
-
 	s3Creds, s3Config, sess, ts := test.SetupS3(t, "")
 	defer ts.Close()
 
@@ -332,18 +317,9 @@ func TestSaveFileWithS3WorkhorseClient(t *testing.T) {
 	require.NoError(t, err)
 
 	test.S3ObjectExists(t, sess, s3Config, remoteObject, test.ObjectContent)
-
-	require.True(t, testhelper.WaitForLogEvent(logHook))
-	entries := logHook.AllEntries()
-	require.Equal(t, 1, len(entries))
-	msg := entries[0].Message
-	require.Contains(t, msg, "saved file")
-	require.Contains(t, msg, "client_mode=s3")
 }
 
 func TestSaveFileWithAzureWorkhorseClient(t *testing.T) {
-	logHook := testhelper.SetupLogger()
-
 	mux, bucketDir, cleanup := test.SetupGoCloudFileBucket(t, "azblob")
 	defer cleanup()
 
@@ -367,13 +343,6 @@ func TestSaveFileWithAzureWorkhorseClient(t *testing.T) {
 	require.NoError(t, err)
 
 	test.GoCloudObjectExists(t, bucketDir, remoteObject)
-
-	require.True(t, testhelper.WaitForLogEvent(logHook))
-	entries := logHook.AllEntries()
-	require.Equal(t, 1, len(entries))
-	msg := entries[0].Message
-	require.Contains(t, msg, "saved file")
-	require.Contains(t, msg, "client_mode=\"go_cloud:AzureRM\"")
 }
 
 func TestSaveFileWithUnknownGoCloudScheme(t *testing.T) {

internal/objectstore/gocloud_object_test.go

@@ -16,8 +16,6 @@ import (
 )
 
 func TestGoCloudObjectUpload(t *testing.T) {
-	logHook := testhelper.SetupLogger()
-
 	mux, _, cleanup := test.SetupGoCloudFileBucket(t, "azuretest")
 	defer cleanup()
 
@@ -60,9 +58,4 @@ func TestGoCloudObjectUpload(t *testing.T) {
 			return nil
 		}
 	})
-
-	// Verify no log noise when deleting a file that already is gone
-	object.Delete()
-	entries := logHook.AllEntries()
-	require.Equal(t, 0, len(entries))
 }

internal/testhelper/testhelper.go

@@ -15,8 +15,6 @@ import (
 	"time"
 
 	"github.com/dgrijalva/jwt-go"
-	"github.com/sirupsen/logrus"
-	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/labkit/log"
@@ -140,28 +138,6 @@ type UploadClaims struct {
 	jwt.StandardClaims
 }
 
-func SetupLogger() *test.Hook {
-	logger, hook := test.NewNullLogger()
-	logrus.SetOutput(logger.Writer())
-
-	return hook
-}
-
-// logrus fires a Goroutine to write the output log, but there's no way to
-// flush all outstanding hooks to fire. We just wait up to a second
-// for an event to appear.
-func WaitForLogEvent(hook *test.Hook) bool {
-	for i := 0; i < 10; i++ {
-		if entry := hook.LastEntry(); entry != nil {
-			return true
-		}
-
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	return false
-}
-
 func Retry(t testing.TB, timeout time.Duration, fn func() error) {
 	t.Helper()
 	start := time.Now()