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.

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 {

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()

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)
+}

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