Add tests for the `platform server` command (#8231)

* Cleanup app state on initialization error

When returning an initialization error, the app state was not cleaned
up. This is especially visible during tests, as `appCount` is not
decremented, and makes the new app initialization fail.

* Test the `platform server` command

As the `platform server` command only exits when interrupted by
a signal, it is not possible to test it as the other cobra
commands. Instead we directly test the actual command function.

The internal command handler is slighly refactored to take
a channel in argument, and registers it as the signal handler.
Nothing very different—except than controlling this channel
from the outside allows the test to send the system signal
itself, thus preventing the server to run forever.

4 files changed, 89 insertions, 10 deletions

diff --git a/app/app.go b/app/app.go
index dd5deb342..636f0a428 100644
--- a/app/app.go
+++ b/app/app.go
@@ -86,7 +86,7 @@ var appCount = 0
 
 // New creates a new App. You must call Shutdown when you're done with it.
 // XXX: For now, only one at a time is allowed as some resources are still shared.
-func New(options ...Option) (*App, error) {
+func New(options ...Option) (outApp *App, outErr error) {
 	appCount++
 	if appCount > 1 {
 		panic("Only one App should exist at a time. Did you forget to call Shutdown()?")
@@ -103,6 +103,11 @@ func New(options ...Option) (*App, error) {
 		clientConfig:     make(map[string]string),
 		licenseListeners: map[string]func(){},
 	}
+	defer func() {
+		if outErr != nil {
+			app.Shutdown()
+		}
+	}()
 
 	for _, option := range options {
 		option(app)
@@ -182,7 +187,9 @@ func (a *App) Shutdown() {
 	a.ShutDownPlugins()
 	a.WaitForGoroutines()
 
-	a.Srv.Store.Close()
+	if a.Srv.Store != nil {
+		a.Srv.Store.Close()
+	}
 	a.Srv = nil
 
 	if a.htmlTemplateWatcher != nil {
diff --git a/cmd/platform/server.go b/cmd/platform/server.go
index a8a6e8923..1b411cf20 100644
--- a/cmd/platform/server.go
+++ b/cmd/platform/server.go
@@ -42,10 +42,11 @@ func runServerCmd(cmd *cobra.Command, args []string) error {
 
 	disableConfigWatch, _ := cmd.Flags().GetBool("disableconfigwatch")
 
-	return runServer(config, disableConfigWatch)
+	interruptChan := make(chan os.Signal, 1)
+	return runServer(config, disableConfigWatch, interruptChan)
 }
 
-func runServer(configFileLocation string, disableConfigWatch bool) error {
+func runServer(configFileLocation string, disableConfigWatch bool, interruptChan chan os.Signal) error {
 	options := []app.Option{app.ConfigFile(configFileLocation)}
 	if disableConfigWatch {
 		options = append(options, app.DisableConfigWatch)
@@ -165,9 +166,8 @@ func runServer(configFileLocation string, disableConfigWatch bool) error {
 
 	// wait for kill signal before attempting to gracefully shutdown
 	// the running service
-	c := make(chan os.Signal, 1)
-	signal.Notify(c, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
-	<-c
+	signal.Notify(interruptChan, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
+	<-interruptChan
 
 	if a.Cluster != nil {
 		a.Cluster.StopInterNodeCommunication()
diff --git a/cmd/platform/server_test.go b/cmd/platform/server_test.go
new file mode 100644
index 000000000..15f9a357a
--- /dev/null
+++ b/cmd/platform/server_test.go
@@ -0,0 +1,72 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"io/ioutil"
+	"os"
+	"syscall"
+	"testing"
+
+	"github.com/mattermost/mattermost-server/jobs"
+	"github.com/mattermost/mattermost-server/utils"
+	"github.com/stretchr/testify/require"
+)
+
+type ServerTestHelper struct {
+	configPath         string
+	disableConfigWatch bool
+	interruptChan      chan os.Signal
+	originalInterval   int
+}
+
+func SetupServerTest() *ServerTestHelper {
+	// Build a channel that will be used by the server to receive system signals…
+	interruptChan := make(chan os.Signal, 1)
+	// …and sent it immediately a SIGINT value.
+	// This will make the server loop stop as soon as it started successfully.
+	interruptChan <- syscall.SIGINT
+
+	// Let jobs poll for termination every 0.2s (instead of every 15s by default)
+	// Otherwise we would have to wait the whole polling duration before the test
+	// terminates.
+	originalInterval := jobs.DEFAULT_WATCHER_POLLING_INTERVAL
+	jobs.DEFAULT_WATCHER_POLLING_INTERVAL = 200
+
+	th := &ServerTestHelper{
+		configPath:         utils.FindConfigFile("config.json"),
+		disableConfigWatch: true,
+		interruptChan:      interruptChan,
+		originalInterval:   originalInterval,
+	}
+	return th
+}
+
+func (th *ServerTestHelper) TearDownServerTest() {
+	jobs.DEFAULT_WATCHER_POLLING_INTERVAL = th.originalInterval
+}
+
+func TestRunServerSuccess(t *testing.T) {
+	th := SetupServerTest()
+	defer th.TearDownServerTest()
+
+	err := runServer(th.configPath, th.disableConfigWatch, th.interruptChan)
+	require.NoError(t, err)
+}
+
+func TestRunServerInvalidConfigFile(t *testing.T) {
+	th := SetupServerTest()
+	defer th.TearDownServerTest()
+
+	// Start the server with an unreadable config file
+	unreadableConfigFile, err := ioutil.TempFile("", "mattermost-unreadable-config-file-")
+	if err != nil {
+		panic(err)
+	}
+	os.Chmod(unreadableConfigFile.Name(), 0200)
+	defer os.Remove(unreadableConfigFile.Name())
+
+	err = runServer(unreadableConfigFile.Name(), th.disableConfigWatch, th.interruptChan)
+	require.Error(t, err)
+}
diff --git a/jobs/jobs_watcher.go b/jobs/jobs_watcher.go
index f519e7cca..eaa3a4e73 100644
--- a/jobs/jobs_watcher.go
+++ b/jobs/jobs_watcher.go
@@ -11,9 +11,9 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
-const (
-	DEFAULT_WATCHER_POLLING_INTERVAL = 15000
-)
+// Default polling interval for jobs termination.
+// (Defining as `var` rather than `const` allows tests to lower the interval.)
+var DEFAULT_WATCHER_POLLING_INTERVAL = 15000
 
 type Watcher struct {
 	srv     *JobServer