From d0d6125541fa5240c8e65173140d10312a5e2bdb Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 17 Mar 2020 21:26:59 +0530 Subject: [PATCH] MM-2276: Fix flaky TestPlugin in cmd/mattermost/commands (#14064) There was a race in (*App).SyncPlugins where if the same plugin existed in availablePlugins and pluginSignaturePathMap, then we would try to add/remove at the same time. This would lead to a possible removal / addition of a plugin directory or even unable to remove a directory because it was already in use. We fix this by first finishing the removal of availablePlugins before syncing it with the file store. --- app/plugin.go | 2 +- cmd/mattermost/commands/plugin_test.go | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/app/plugin.go b/app/plugin.go index b9d99e7cdb..308bf14bec 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -230,6 +230,7 @@ func (a *App) SyncPlugins() *model.AppError { } }(plugin.Manifest.Id) } + wg.Wait() // Install plugins from the file store. pluginSignaturePathMap, appErr := a.getPluginsFromFolder() @@ -263,7 +264,6 @@ func (a *App) SyncPlugins() *model.AppError { mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(err)) } }(plugin) - } wg.Wait() diff --git a/cmd/mattermost/commands/plugin_test.go b/cmd/mattermost/commands/plugin_test.go index 25a3cc228c..71760eb79a 100644 --- a/cmd/mattermost/commands/plugin_test.go +++ b/cmd/mattermost/commands/plugin_test.go @@ -23,22 +23,29 @@ func TestPlugin(t *testing.T) { *cfg.PluginSettings.ClientDirectory = "./test-client-plugins" th.SetConfig(cfg) - os.MkdirAll("./test-plugins", os.ModePerm) - os.MkdirAll("./test-client-plugins", os.ModePerm) + err := os.MkdirAll("./test-plugins", os.ModePerm) + require.Nil(t, err) + err = os.MkdirAll("./test-client-plugins", os.ModePerm) + require.Nil(t, err) path, _ := fileutils.FindDir("tests") - th.CheckCommand(t, "plugin", "add", filepath.Join(path, "testplugin.tar.gz")) + output := th.CheckCommand(t, "plugin", "add", filepath.Join(path, "testplugin.tar.gz")) + assert.Contains(t, output, "Added plugin:") + output = th.CheckCommand(t, "plugin", "enable", "testplugin") + assert.Contains(t, output, "Enabled plugin: testplugin") - th.CheckCommand(t, "plugin", "enable", "testplugin") fs, err := config.NewFileStore(th.ConfigPath(), false) require.Nil(t, err) + require.NotNil(t, fs.Get().PluginSettings.PluginStates["testplugin"]) assert.True(t, fs.Get().PluginSettings.PluginStates["testplugin"].Enable) fs.Close() - th.CheckCommand(t, "plugin", "disable", "testplugin") + output = th.CheckCommand(t, "plugin", "disable", "testplugin") + assert.Contains(t, output, "Disabled plugin: testplugin") fs, err = config.NewFileStore(th.ConfigPath(), false) require.Nil(t, err) + require.NotNil(t, fs.Get().PluginSettings.PluginStates["testplugin"]) assert.False(t, fs.Get().PluginSettings.PluginStates["testplugin"].Enable) fs.Close()