From 756f5fbff355cb2776c9a5dc55bee4a8813d62a9 Mon Sep 17 00:00:00 2001
From: Agniva De Sarker <agnivade@yahoo.co.in>
Date: Thu, 1 Apr 2021 00:33:49 +0530
Subject: [PATCH] MM-34086: Fix flaky test StartServerTLSOverwriteCipher
 (#17303)

* MM-34086: Fix flaky test StartServerTLSOverwriteCipher

This suffered from the same race condition as
https://github.com/mattermost/mattermost-server/pull/17215.

We apply the same approach of using a memstore instead
of a file based config store.

It is likely that the test was also failing because of
the same race. Although I don't obviously see how, because
the race happens with mlog.Info. The test will fail only
if the config wasn't able to set properly. So although
not strictly related, it's somehow related.

If it fails again, we can look into it again. But this
atleast fixes the race.

While here, we also apply some cleanup of the code.

https://mattermost.atlassian.net/browse/MM-34086

```release-note
NONE
```

* Remove race test
---
 app/server_test.go | 47 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/app/server_test.go b/app/server_test.go
index 13e356112b..4ab7c46db1 100644
--- a/app/server_test.go
+++ b/app/server_test.go
@@ -431,23 +431,29 @@ func TestStartServerTLSVersion(t *testing.T) {
 }
 
 func TestStartServerTLSOverwriteCipher(t *testing.T) {
-	t.Skip("Flaky test: MM-34086")
+	configStore, _ := config.NewMemoryStore()
+	store, _ := config.NewStoreFromBacking(configStore, nil, false)
+	cfg := store.Get()
+	testDir, _ := fileutils.FindDir("tests")
 
-	s, err := NewServer()
+	*cfg.ServiceSettings.ListenAddress = ":0"
+	*cfg.ServiceSettings.ConnectionSecurity = "TLS"
+	cfg.ServiceSettings.TLSOverwriteCiphers = []string{
+		"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+		"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+	}
+	*cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem")
+	*cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem")
+
+	store.Set(cfg)
+
+	s, err := NewServer(ConfigStore(store))
 	require.NoError(t, err)
 
-	testDir, _ := fileutils.FindDir("tests")
-	s.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.ListenAddress = ":0"
-		*cfg.ServiceSettings.ConnectionSecurity = "TLS"
-		cfg.ServiceSettings.TLSOverwriteCiphers = []string{
-			"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
-			"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
-		}
-		*cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem")
-		*cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem")
-	})
-	serverErr := s.Start()
+	err = s.Start()
+	require.NoError(t, err)
+
+	defer s.Shutdown()
 
 	tr := &http.Transport{
 		TLSClientConfig: &tls.Config{
@@ -462,9 +468,7 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) {
 	client := &http.Client{Transport: tr}
 	err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/")
 	require.Error(t, err, "Expected error due to Cipher mismatch")
-	if !strings.Contains(err.Error(), "remote error: tls: handshake failure") {
-		t.Errorf("Expected protocol version error, got %s", err)
-	}
+	require.Contains(t, err.Error(), "remote error: tls: handshake failure", "Expected protocol version error")
 
 	client.Transport = &http.Transport{
 		TLSClientConfig: &tls.Config{
@@ -478,18 +482,11 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) {
 	}
 
 	err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/")
-
-	if err != nil {
-		t.Errorf("Expected nil, got %s", err)
-	}
-
-	s.Shutdown()
-	require.NoError(t, serverErr)
+	require.NoError(t, err)
 }
 
 func checkEndpoint(t *testing.T, client *http.Client, url string) error {
 	res, err := client.Get(url)
-
 	if err != nil {
 		return err
 	}