diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index 4a1807caa2a..eb151eca511 100644 --- a/pkg/api/login_test.go +++ b/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{}, diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index a28ec025750..9e869285efd 100644 --- a/pkg/api/plugins_test.go +++ b/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 diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index 97621f59056..09395b036af 100644 --- a/pkg/api/team_test.go +++ b/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.Equal(t, stub.warnMessage, "Could not add creator to team because is not a real user") + assert.NotZero(t, logger.WarnLogs.Calls) + 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) }) }) } diff --git a/pkg/infra/log/logtest/fake.go b/pkg/infra/log/logtest/fake.go new file mode 100644 index 00000000000..a65f6789382 --- /dev/null +++ b/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 +} diff --git a/pkg/plugins/manager/loader/initializer/initializer_test.go b/pkg/plugins/manager/loader/initializer/initializer_test.go index d63a34384c3..b9281b7282e 100644 --- a/pkg/plugins/manager/loader/initializer/initializer_test.go +++ b/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 diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index ec523fe3646..b7f399c06f3 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/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) -} diff --git a/pkg/plugins/manager/manager_test.go b/pkg/plugins/manager/manager_test.go index b18609f6da9..9b5435fafcb 100644 --- a/pkg/plugins/manager/manager_test.go +++ b/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 } diff --git a/pkg/services/updatechecker/plugins_test.go b/pkg/services/updatechecker/plugins_test.go index 1f89033f083..29cb69e268b 100644 --- a/pkg/services/updatechecker/plugins_test.go +++ b/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{}) {} diff --git a/pkg/setting/setting_session_test.go b/pkg/setting/setting_session_test.go index 7ad622b02e7..07554d87885 100644 --- a/pkg/setting/setting_session_test.go +++ b/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{} - cfg.Logger = stub + logger := &logtest.Fake{} + 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.Greater(t, len(stub.warnMessage), 0) + require.Equal(t, 1, logger.WarnLogs.Calls) + require.Greater(t, len(logger.WarnLogs.Message), 0) }) }