From 857e18f3ab468068c079764b04608b21b38ddbb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 9 Mar 2021 09:45:00 +0100 Subject: [PATCH] Replacing require.nil in plugin package (#16961) Co-authored-by: Mattermod --- plugin/health_check_test.go | 10 +++---- plugin/helpers_bots.go | 7 +++-- plugin/helpers_bots_test.go | 54 +++++++++++++++++------------------ plugin/helpers_config_test.go | 4 +-- plugin/helpers_kv_test.go | 20 ++++++------- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/plugin/health_check_test.go b/plugin/health_check_test.go index 3659ba2d17..7aa559576c 100644 --- a/plugin/health_check_test.go +++ b/plugin/health_check_test.go @@ -60,12 +60,12 @@ func testPluginHealthCheckSuccess(t *testing.T) { }) supervisor, err := newSupervisor(bundle, nil, log, nil) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, supervisor) defer supervisor.Shutdown() err = supervisor.PerformHealthCheck() - require.Nil(t, err) + require.NoError(t, err) } func testPluginHealthCheckPanic(t *testing.T) { @@ -107,17 +107,17 @@ func testPluginHealthCheckPanic(t *testing.T) { }) supervisor, err := newSupervisor(bundle, nil, log, nil) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, supervisor) defer supervisor.Shutdown() err = supervisor.PerformHealthCheck() - require.Nil(t, err) + require.NoError(t, err) supervisor.hooks.MessageWillBePosted(&Context{}, &model.Post{}) err = supervisor.PerformHealthCheck() - require.NotNil(t, err) + require.Error(t, err) } func TestShouldDeactivatePlugin(t *testing.T) { diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index ed30cb62fc..8b964ae657 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -212,9 +212,10 @@ func (p *HelpersImpl) ensureBot(bot *model.Bot) (retBotID string, retErr error) var botIDBytes []byte err = utils.ProgressiveRetry(func() error { - botIDBytes, err = p.API.KVGet(BotUserKey) - if err != nil { - return err + var appErr *model.AppError + botIDBytes, appErr = p.API.KVGet(BotUserKey) + if appErr != nil { + return appErr } return nil }) diff --git a/plugin/helpers_bots_test.go b/plugin/helpers_bots_test.go index 850c9f9ab4..1d4ec4d43b 100644 --- a/plugin/helpers_bots_test.go +++ b/plugin/helpers_bots_test.go @@ -38,7 +38,7 @@ func TestEnsureBot(t *testing.T) { _, retErr := p.EnsureBot(nil) - assert.NotNil(t, retErr) + assert.Error(t, retErr) assert.Equal(t, "failed to ensure bot: incompatible server version for plugin, minimum required version: 5.10.0, current version: 5.9.0", retErr.Error()) }) @@ -51,7 +51,7 @@ func TestEnsureBot(t *testing.T) { p.API = api botID, err := p.EnsureBot(nil) assert.Equal(t, "", botID) - assert.NotNil(t, err) + assert.Error(t, err) }) t.Run("bad username", func(t *testing.T) { api := setupAPI() @@ -63,7 +63,7 @@ func TestEnsureBot(t *testing.T) { Username: "", }) assert.Equal(t, "", botID) - assert.NotNil(t, err) + assert.Error(t, err) }) }) @@ -87,7 +87,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should return an error if unable to get bot", func(t *testing.T) { @@ -102,7 +102,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot) assert.Equal(t, "", botID) - assert.NotNil(t, err) + assert.Error(t, err) }) t.Run("should set the bot profile image when specified", func(t *testing.T) { @@ -127,11 +127,11 @@ func TestEnsureBot(t *testing.T) { p := &plugin.HelpersImpl{} p.API = api - assert.Nil(t, err) + assert.NoError(t, err) botID, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should set the bot icon image when specified", func(t *testing.T) { @@ -141,7 +141,7 @@ func TestEnsureBot(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") imageBytes, err := ioutil.ReadFile(testImage) - assert.Nil(t, err) + assert.NoError(t, err) api.On("KVGet", plugin.BotUserKey).Return([]byte(expectedBotID), nil) api.On("GetBundlePath").Return("", nil) @@ -159,7 +159,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot, plugin.IconImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should set both the profile image and bot icon image when specified", func(t *testing.T) { @@ -169,7 +169,7 @@ func TestEnsureBot(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") imageBytes, err := ioutil.ReadFile(testImage) - assert.Nil(t, err) + assert.NoError(t, err) api.On("KVGet", plugin.BotUserKey).Return([]byte(expectedBotID), nil) api.On("GetBundlePath").Return("", nil) @@ -188,7 +188,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage), plugin.IconImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should find and update the bot with new bot details", func(t *testing.T) { @@ -201,7 +201,7 @@ func TestEnsureBot(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") imageBytes, err := ioutil.ReadFile(testImage) - assert.Nil(t, err) + assert.NoError(t, err) api := setupAPI() api.On("GetServerVersion").Return("5.10.0") @@ -227,7 +227,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(updatedTestbot, plugin.ProfileImagePath(testImage), plugin.IconImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) }) @@ -251,7 +251,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should claim existing bot and return the ID", func(t *testing.T) { @@ -273,7 +273,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should return the non-bot account but log a message if user exists with the same name and is not a bot", func(t *testing.T) { @@ -294,7 +294,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should fail if create bot fails", func(t *testing.T) { @@ -311,7 +311,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot) assert.Equal(t, "", botID) - assert.NotNil(t, err) + assert.Error(t, err) }) t.Run("should create bot and set the bot profile image when specified", func(t *testing.T) { @@ -321,7 +321,7 @@ func TestEnsureBot(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") imageBytes, err := ioutil.ReadFile(testImage) - assert.Nil(t, err) + assert.NoError(t, err) api.On("KVGet", plugin.BotUserKey).Return(nil, nil) api.On("GetUserByUsername", testbot.Username).Return(nil, nil) @@ -339,7 +339,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should create bot and set the bot icon image when specified", func(t *testing.T) { @@ -349,7 +349,7 @@ func TestEnsureBot(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") imageBytes, err := ioutil.ReadFile(testImage) - assert.Nil(t, err) + assert.NoError(t, err) api.On("KVGet", plugin.BotUserKey).Return(nil, nil) api.On("GetUserByUsername", testbot.Username).Return(nil, nil) @@ -367,7 +367,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot, plugin.IconImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should create bot and set both the profile image and bot icon image when specified", func(t *testing.T) { @@ -377,7 +377,7 @@ func TestEnsureBot(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") imageBytes, err := ioutil.ReadFile(testImage) - assert.Nil(t, err) + assert.NoError(t, err) api.On("KVGet", plugin.BotUserKey).Return(nil, nil) api.On("GetUserByUsername", testbot.Username).Return(nil, nil) @@ -396,7 +396,7 @@ func TestEnsureBot(t *testing.T) { botID, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage), plugin.IconImagePath(testImage)) assert.Equal(t, expectedBotID, botID) - assert.Nil(t, err) + assert.NoError(t, err) }) }) } @@ -518,7 +518,7 @@ func TestShouldProcessMessage(t *testing.T) { shouldProcessMessage, err := p.ShouldProcessMessage(&model.Post{ChannelId: channelID, Props: model.StringInterface{"from_webhook": "true"}}, plugin.AllowBots()) assert.False(t, shouldProcessMessage) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("should process the message which have from_webhook with allow webhook plugin", func(t *testing.T) { @@ -529,7 +529,7 @@ func TestShouldProcessMessage(t *testing.T) { api.On("KVGet", plugin.BotUserKey).Return([]byte(expectedBotID), nil) shouldProcessMessage, err := p.ShouldProcessMessage(&model.Post{ChannelId: channelID, Props: model.StringInterface{"from_webhook": "true"}}, plugin.AllowBots(), plugin.AllowWebhook()) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, shouldProcessMessage) }) @@ -542,7 +542,7 @@ func TestShouldProcessMessage(t *testing.T) { api.On("KVGet", plugin.BotUserKey).Return([]byte(expectedBotID), nil) shouldProcessMessage, err := p.ShouldProcessMessage(&model.Post{ChannelId: channelID}, plugin.AllowBots()) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, shouldProcessMessage) }) @@ -555,7 +555,7 @@ func TestShouldProcessMessage(t *testing.T) { api.On("KVGet", plugin.BotUserKey).Return([]byte(expectedBotID), nil) shouldProcessMessage, err := p.ShouldProcessMessage(&model.Post{ChannelId: channelID, Props: model.StringInterface{"from_webhook": "false"}}, plugin.AllowBots()) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, shouldProcessMessage) }) diff --git a/plugin/helpers_config_test.go b/plugin/helpers_config_test.go index 263e2cf1e5..7ee892286c 100644 --- a/plugin/helpers_config_test.go +++ b/plugin/helpers_config_test.go @@ -114,9 +114,9 @@ func TestCheckRequiredServerConfiguration(t *testing.T) { assert.Equal(t, test.ShouldReturn, ok) if test.ShouldError { - assert.NotNil(t, err) + assert.Error(t, err) } else { - assert.Nil(t, err) + assert.NoError(t, err) } }) } diff --git a/plugin/helpers_kv_test.go b/plugin/helpers_kv_test.go index ae6018a1f4..19725aca49 100644 --- a/plugin/helpers_kv_test.go +++ b/plugin/helpers_kv_test.go @@ -438,7 +438,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions() api.AssertExpectations(t) assert.Empty(t, keys) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("Basic Success, one page", func(t *testing.T) { @@ -452,7 +452,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions() api.AssertExpectations(t) assert.ElementsMatch(t, keys, []string{"key1", "key2"}) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("Basic Success, two page", func(t *testing.T) { @@ -467,7 +467,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions() api.AssertExpectations(t) assert.ElementsMatch(t, keys, getKeys(101)) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("error on second page", func(t *testing.T) { @@ -497,7 +497,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithPrefix("key99")) api.AssertExpectations(t) assert.ElementsMatch(t, keys, []string{"key99"}) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("success, two page, filter prefix, all", func(t *testing.T) { @@ -512,7 +512,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithPrefix("notkey")) api.AssertExpectations(t) assert.Empty(t, keys) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("success, two page, filter prefix, none", func(t *testing.T) { @@ -527,7 +527,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithPrefix("key")) api.AssertExpectations(t) assert.ElementsMatch(t, keys, getKeys(101)) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("success, two page, checker func, one", func(t *testing.T) { @@ -549,7 +549,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithChecker(check)) api.AssertExpectations(t) assert.ElementsMatch(t, keys, []string{"key1"}) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("success, two page, checker func, all", func(t *testing.T) { @@ -568,7 +568,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithChecker(check)) api.AssertExpectations(t) assert.Empty(t, keys) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("success, two page, checker func, none", func(t *testing.T) { @@ -587,7 +587,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithChecker(check)) api.AssertExpectations(t) assert.ElementsMatch(t, keys, getKeys(101)) - assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("error, checker func", func(t *testing.T) { @@ -626,7 +626,7 @@ func TestKVListWithOptions(t *testing.T) { keys, err := p.KVListWithOptions(plugin.WithPrefix("key"), plugin.WithChecker(check)) api.AssertExpectations(t) assert.ElementsMatch(t, keys, []string{"key2", "key4"}) - assert.Nil(t, err) + assert.NoError(t, err) }) }