Logging: Unify logging fakes (#48822)

pkg/api/login_test.go

@@ -13,6 +13,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/api/routing"
@@ -28,8 +31,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/secrets/fakes"
 	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func fakeSetIndexViewData(t *testing.T) {
@@ -65,22 +66,6 @@ func getBody(resp *httptest.ResponseRecorder) (string, error) {
 	return string(responseData), nil
 }
 
-type FakeLogger struct {
-	log.Logger
-}
-
-func (fl *FakeLogger) Debug(testMessage string, ctx ...interface{}) {
-}
-
-func (fl *FakeLogger) Info(testMessage string, ctx ...interface{}) {
-}
-
-func (fl *FakeLogger) Warn(testMessage string, ctx ...interface{}) {
-}
-
-func (fl *FakeLogger) Error(testMessage string, ctx ...interface{}) {
-}
-
 type redirectCase struct {
 	desc        string
 	url         string
@@ -332,7 +317,7 @@ func TestLoginPostRedirect(t *testing.T) {
 	fakeViewIndex(t)
 	sc := setupScenarioContext(t, "/login")
 	hs := &HTTPServer{
-		log:              &FakeLogger{},
+		log:              log.NewNopLogger(),
 		Cfg:              setting.NewCfg(),
 		HooksService:     &hooks.HooksService{},
 		License:          &licensing.OSSLicensingService{},

pkg/api/plugins_test.go

@@ -11,6 +11,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -55,7 +57,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				pluginID: p,
 			},
 		}
-		l := &logger{}
+		l := &logtest.Fake{}
 
 		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
 		pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l,
@@ -64,7 +66,7 @@ func Test_GetPluginAssets(t *testing.T) {
 
 				require.Equal(t, 200, sc.resp.Code)
 				assert.Equal(t, expectedBody, sc.resp.Body.String())
-				assert.Empty(t, l.warnings)
+				assert.Zero(t, l.WarnLogs.Calls)
 			})
 	})
 
@@ -80,7 +82,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				pluginID: p,
 			},
 		}
-		l := &logger{}
+		l := &logtest.Fake{}
 
 		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, tmpFileInParentDir.Name())
 		pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l,
@@ -103,7 +105,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				pluginID: p,
 			},
 		}
-		l := &logger{}
+		l := &logtest.Fake{}
 
 		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
 		pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l,
@@ -112,7 +114,7 @@ func Test_GetPluginAssets(t *testing.T) {
 
 				require.Equal(t, 200, sc.resp.Code)
 				assert.Equal(t, expectedBody, sc.resp.Body.String())
-				assert.Empty(t, l.warnings)
+				assert.Zero(t, l.WarnLogs.Calls)
 			})
 	})
 
@@ -128,7 +130,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				pluginID: p,
 			},
 		}
-		l := &logger{}
+		l := &logtest.Fake{}
 
 		requestedFile := "nonExistent"
 		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
@@ -141,7 +143,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, 404, sc.resp.Code)
 				assert.Equal(t, "Plugin file not found", respJson["message"])
-				assert.Empty(t, l.warnings)
+				assert.Zero(t, l.WarnLogs.Calls)
 			})
 	})
 
@@ -149,7 +151,7 @@ func Test_GetPluginAssets(t *testing.T) {
 		service := &fakePluginStore{
 			plugins: map[string]plugins.PluginDTO{},
 		}
-		l := &logger{}
+		l := &logtest.Fake{}
 
 		requestedFile := "nonExistent"
 		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
@@ -162,7 +164,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				require.NoError(t, err)
 				assert.Equal(t, 404, sc.resp.Code)
 				assert.Equal(t, "Plugin not found", respJson["message"])
-				assert.Empty(t, l.warnings)
+				assert.Zero(t, l.WarnLogs.Calls)
 			})
 	})
 
@@ -174,7 +176,7 @@ func Test_GetPluginAssets(t *testing.T) {
 				},
 			},
 		}
-		l := &logger{}
+		l := &logtest.Fake{}
 
 		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
 		pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l,
@@ -183,7 +185,7 @@ func Test_GetPluginAssets(t *testing.T) {
 
 				require.Equal(t, 200, sc.resp.Code)
 				assert.Equal(t, expectedBody, sc.resp.Body.String())
-				assert.Empty(t, l.warnings)
+				assert.Zero(t, l.WarnLogs.Calls)
 			})
 	})
 }
@@ -235,16 +237,6 @@ func pluginAssetScenario(t *testing.T, desc string, url string, urlPattern strin
 	})
 }
 
-type logger struct {
-	log.Logger
-
-	warnings []string
-}
-
-func (l *logger) Warn(msg string, ctx ...interface{}) {
-	l.warnings = append(l.warnings, msg)
-}
-
 type fakePluginClient struct {
 	plugins.Client
 

pkg/api/team_test.go

@@ -8,7 +8,11 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	pref "github.com/grafana/grafana/pkg/services/preference"
@@ -17,21 +21,8 @@ import (
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
-type testLogger struct {
-	log.Logger
-	warnCalled  bool
-	warnMessage string
-}
-
-func (stub *testLogger) Warn(testMessage string, ctx ...interface{}) {
-	stub.warnCalled = true
-	stub.warnMessage = testMessage
-}
-
 func TestTeamAPIEndpoint(t *testing.T) {
 	t.Run("Given two teams", func(t *testing.T) {
 		hs := setupSimpleHTTPServer(nil)
@@ -123,11 +114,11 @@ func TestTeamAPIEndpoint(t *testing.T) {
 		require.NoError(t, err)
 
 		t.Run("with no real signed in user", func(t *testing.T) {
-			stub := &testLogger{}
+			logger := &logtest.Fake{}
 			c := &models.ReqContext{
 				Context:      &web.Context{Req: req},
 				SignedInUser: &models.SignedInUser{},
-				Logger:       stub,
+				Logger:       logger,
 			}
 			c.OrgRole = models.ROLE_EDITOR
 			c.Req.Body = mockRequestBody(models.CreateTeamCommand{Name: teamName})
@@ -135,23 +126,23 @@ func TestTeamAPIEndpoint(t *testing.T) {
 			r := hs.CreateTeam(c)
 
 			assert.Equal(t, 200, r.Status())
-			assert.True(t, stub.warnCalled)
+			assert.NotZero(t, logger.WarnLogs.Calls)
-			assert.Equal(t, stub.warnMessage, "Could not add creator to team because is not a real user")
+			assert.Equal(t, "Could not add creator to team because is not a real user", logger.WarnLogs.Message)
 		})
 
 		t.Run("with real signed in user", func(t *testing.T) {
-			stub := &testLogger{}
+			logger := &logtest.Fake{}
 			c := &models.ReqContext{
 				Context:      &web.Context{Req: req},
 				SignedInUser: &models.SignedInUser{UserId: 42},
-				Logger:       stub,
+				Logger:       logger,
 			}
 			c.OrgRole = models.ROLE_EDITOR
 			c.Req.Body = mockRequestBody(models.CreateTeamCommand{Name: teamName})
 			c.Req.Header.Add("Content-Type", "application/json")
 			r := hs.CreateTeam(c)
 			assert.Equal(t, 200, r.Status())
-			assert.False(t, stub.warnCalled)
+			assert.Zero(t, logger.WarnLogs.Calls)
 		})
 	})
 }

pkg/infra/log/logtest/fake.go

@@ -0,0 +1,50 @@
+package logtest
+
+import (
+	"github.com/grafana/grafana/pkg/infra/log"
+)
+
+type Fake struct {
+	DebugLogs Logs
+	InfoLogs  Logs
+	WarnLogs  Logs
+	ErrorLogs Logs
+}
+
+type Logs struct {
+	Calls   int
+	Message string
+	Ctx     []interface{}
+}
+
+func (f *Fake) New(ctx ...interface{}) *log.ConcreteLogger {
+	return log.NewNopLogger()
+}
+
+func (f *Fake) Log(keyvals ...interface{}) error {
+	return nil
+}
+
+func (f *Fake) Debug(msg string, ctx ...interface{}) {
+	f.DebugLogs.Calls++
+	f.DebugLogs.Message = msg
+	f.DebugLogs.Ctx = ctx
+}
+
+func (f *Fake) Info(msg string, ctx ...interface{}) {
+	f.InfoLogs.Calls++
+	f.InfoLogs.Message = msg
+	f.InfoLogs.Ctx = ctx
+}
+
+func (f *Fake) Warn(msg string, ctx ...interface{}) {
+	f.WarnLogs.Calls++
+	f.WarnLogs.Message = msg
+	f.WarnLogs.Ctx = ctx
+}
+
+func (f *Fake) Error(msg string, ctx ...interface{}) {
+	f.ErrorLogs.Calls++
+	f.ErrorLogs.Message = msg
+	f.ErrorLogs.Ctx = ctx
+}

pkg/plugins/manager/loader/initializer/initializer_test.go

@@ -35,7 +35,7 @@ func TestInitializer_Initialize(t *testing.T) {
 
 		i := &Initializer{
 			cfg: plugins.NewCfg(),
-			log: &fakeLogger{},
+			log: log.NewNopLogger(),
 			backendProvider: &fakeBackendProvider{
 				plugin: p,
 			},
@@ -65,7 +65,7 @@ func TestInitializer_Initialize(t *testing.T) {
 
 		i := &Initializer{
 			cfg: plugins.NewCfg(),
-			log: fakeLogger{},
+			log: log.NewNopLogger(),
 			backendProvider: &fakeBackendProvider{
 				plugin: p,
 			},
@@ -88,7 +88,7 @@ func TestInitializer_Initialize(t *testing.T) {
 
 		i := &Initializer{
 			cfg: &plugins.Cfg{},
-			log: fakeLogger{},
+			log: log.NewNopLogger(),
 			backendProvider: &fakeBackendProvider{
 				plugin: p,
 			},
@@ -126,7 +126,7 @@ func TestInitializer_envVars(t *testing.T) {
 				},
 			},
 			license: licensing,
-			log:     fakeLogger{},
+			log:     log.NewNopLogger(),
 			backendProvider: &fakeBackendProvider{
 				plugin: p,
 			},
@@ -211,18 +211,6 @@ func (*testLicensingService) FeatureEnabled(feature string) bool {
 	return false
 }
 
-type fakeLogger struct {
-	*log.ConcreteLogger
-}
-
-func (f fakeLogger) New(_ ...interface{}) *log.ConcreteLogger {
-	return &log.ConcreteLogger{}
-}
-
-func (f fakeLogger) Warn(_ string, _ ...interface{}) {
-
-}
-
 type fakeBackendProvider struct {
 	plugins.BackendFactoryProvider
 

pkg/plugins/manager/loader/loader_test.go

@@ -7,12 +7,13 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/backendplugin"
@@ -422,13 +423,14 @@ func TestLoader_setDefaultNavURL(t *testing.T) {
 				},
 			}},
 		}
-		logger := &fakeLogger{loggedLines: []string{}}
+		logger := &logtest.Fake{}
 		pluginWithDashboard.SetLogger(logger)
 
 		t.Run("Default nav URL is not set if dashboard UID field not is set", func(t *testing.T) {
 			setDefaultNavURL(pluginWithDashboard)
 			require.Equal(t, "", pluginWithDashboard.DefaultNavURL)
-			require.Equal(t, []string{"Included dashboard is missing a UID field"}, logger.loggedLines)
+			require.NotZero(t, logger.WarnLogs.Calls)
+			require.Equal(t, "Included dashboard is missing a UID field", logger.WarnLogs.Message)
 		})
 
 		t.Run("Default nav URL is set if dashboard UID field is set", func(t *testing.T) {
@@ -1137,7 +1139,7 @@ func newLoader(cfg *plugins.Cfg) *Loader {
 		pluginInitializer:  initializer.New(cfg, provider.ProvideService(coreplugin.NewRegistry(make(map[string]backendplugin.PluginFactoryFunc))), &fakeLicensingService{}),
 		signatureValidator: signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)),
 		errs:               make(map[string]*plugins.SignatureError),
-		log:                &fakeLogger{},
+		log:                &logtest.Fake{},
 	}
 }
 
@@ -1177,25 +1179,3 @@ func (*fakeLicensingService) EnabledFeatures() map[string]bool {
 func (*fakeLicensingService) FeatureEnabled(feature string) bool {
 	return false
 }
-
-type fakeLogger struct {
-	log.Logger
-
-	loggedLines []string
-}
-
-func (fl *fakeLogger) New(_ ...interface{}) *log.ConcreteLogger {
-	return &log.ConcreteLogger{}
-}
-
-func (fl *fakeLogger) Info(l string, _ ...interface{}) {
-	fl.loggedLines = append(fl.loggedLines, l)
-}
-
-func (fl *fakeLogger) Debug(l string, _ ...interface{}) {
-	fl.loggedLines = append(fl.loggedLines, l)
-}
-
-func (fl *fakeLogger) Warn(l string, _ ...interface{}) {
-	fl.loggedLines = append(fl.loggedLines, l)
-}

pkg/plugins/manager/manager_test.go

@@ -10,11 +10,12 @@ import (
 	"github.com/grafana/grafana-azure-sdk-go/azsettings"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/backendplugin"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -544,7 +545,7 @@ func createPlugin(t *testing.T, pluginID, version string, class plugins.Class, m
 		},
 	}
 
-	logger := fakeLogger{}
+	logger := log.NewNopLogger()
 
 	p.SetLogger(logger)
 
@@ -757,18 +758,6 @@ func (pc *fakePluginClient) RunStream(_ context.Context, _ *backend.RunStreamReq
 	return backendplugin.ErrMethodNotImplemented
 }
 
-type fakeLogger struct {
-	log.Logger
-}
-
-func (l fakeLogger) Info(_ string, _ ...interface{}) {
-
-}
-
-func (l fakeLogger) Debug(_ string, _ ...interface{}) {
-
-}
-
 type fakeSender struct {
 	resp *backend.CallResourceResponse
 }

pkg/services/updatechecker/plugins_test.go

@@ -154,7 +154,7 @@ func TestPluginUpdateChecker_checkForUpdates(t *testing.T) {
 			httpClient: &fakeHTTPClient{
 				fakeResp: jsonResp,
 			},
-			log: &fakeLogger{},
+			log: log.NewNopLogger(),
 		}
 
 		svc.checkForUpdates(context.Background())
@@ -215,11 +215,3 @@ func (pr fakePluginStore) Plugins(_ context.Context, _ ...plugins.Type) []plugin
 	}
 	return result
 }
-
-type fakeLogger struct {
-	log.Logger
-}
-
-func (l *fakeLogger) Debug(_ string, _ ...interface{}) {}
-
-func (l *fakeLogger) Warn(_ string, _ ...interface{}) {}

pkg/setting/setting_session_test.go

@@ -4,26 +4,11 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
 
 	"github.com/stretchr/testify/require"
 )
 
-type testLogger struct {
-	log.Logger
-	warnCalled  bool
-	warnMessage string
-}
-
-func (stub *testLogger) Warn(testMessage string, ctx ...interface{}) {
-	stub.warnCalled = true
-	stub.warnMessage = testMessage
-}
-
-func (stub *testLogger) Info(testMessage string, ctx ...interface{}) {
-
-}
-
 func TestSessionSettings(t *testing.T) {
 	skipStaticRootValidation = true
 
@@ -31,8 +16,8 @@ func TestSessionSettings(t *testing.T) {
 		cfg := NewCfg()
 		homePath := "../../"
 
-		stub := &testLogger{}
+		logger := &logtest.Fake{}
-		cfg.Logger = stub
+		cfg.Logger = logger
 
 		err := cfg.Load(CommandLineArgs{
 			HomePath: homePath,
@@ -40,7 +25,7 @@ func TestSessionSettings(t *testing.T) {
 		})
 		require.Nil(t, err)
 
-		require.Equal(t, true, stub.warnCalled)
+		require.Equal(t, 1, logger.WarnLogs.Calls)
-		require.Greater(t, len(stub.warnMessage), 0)
+		require.Greater(t, len(logger.WarnLogs.Message), 0)
 	})
 }