From 6f87eb991f194a8bbc230d3fef086c8bd3dcae80 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 13 Apr 2021 21:12:48 +0530 Subject: [PATCH] MM-34786: Fix racy test TestStartServerTLSSuccess (#17387) * MM-34786: Fix racy test TestStartServerTLSSuccess There were a bunch of more tests which concurrently updated config along with starting the server. We apply the same fix as before in https://github.com/mattermost/mattermost-server/pull/17215 https://mattermost.atlassian.net/browse/MM-34786 ```release-note NONE ``` * Refactor newServer * checking for error too --- app/server_test.go | 119 +++++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 58 deletions(-) diff --git a/app/server_test.go b/app/server_test.go index ecd80c5359..0c07ed8442 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -32,11 +32,25 @@ import ( "github.com/mattermost/mattermost-server/v5/utils/fileutils" ) +func newServerWithConfig(t *testing.T, f func(cfg *model.Config)) (*Server, error) { + configStore, err := config.NewMemoryStore() + require.NoError(t, err) + store, err := config.NewStoreFromBacking(configStore, nil, false) + require.NoError(t, err) + cfg := store.Get() + f(cfg) + + store.Set(cfg) + + return NewServer(ConfigStore(store)) +} + func TestStartServerSuccess(t *testing.T) { - s, err := NewServer() + s, err := newServerWithConfig(t, func(cfg *model.Config) { + *cfg.ServiceSettings.ListenAddress = ":0" + }) require.NoError(t, err) - s.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = ":0" }) serverErr := s.Start() client := &http.Client{} @@ -180,16 +194,16 @@ func TestStartServerNoS3Bucket(t *testing.T) { } func TestStartServerTLSSuccess(t *testing.T) { - s, err := NewServer() - require.NoError(t, err) + s, err := newServerWithConfig(t, func(cfg *model.Config) { + testDir, _ := fileutils.FindDir("tests") - testDir, _ := fileutils.FindDir("tests") - s.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = ":0" *cfg.ServiceSettings.ConnectionSecurity = "TLS" *cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem") *cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem") }) + require.NoError(t, err) + serverErr := s.Start() tr := &http.Transport{ @@ -387,17 +401,22 @@ func TestGenerateSupportPacketYaml(t *testing.T) { } func TestStartServerTLSVersion(t *testing.T) { - s, err := NewServer() + configStore, _ := config.NewMemoryStore() + store, _ := config.NewStoreFromBacking(configStore, nil, false) + cfg := store.Get() + testDir, _ := fileutils.FindDir("tests") + + *cfg.ServiceSettings.ListenAddress = ":0" + *cfg.ServiceSettings.ConnectionSecurity = "TLS" + *cfg.ServiceSettings.TLSMinVer = "1.2" + *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.TLSMinVer = "1.2" - *cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem") - *cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem") - }) serverErr := s.Start() tr := &http.Transport{ @@ -431,23 +450,18 @@ func TestStartServerTLSVersion(t *testing.T) { } func TestStartServerTLSOverwriteCipher(t *testing.T) { - configStore, _ := config.NewMemoryStore() - store, _ := config.NewStoreFromBacking(configStore, nil, false) - cfg := store.Get() - testDir, _ := fileutils.FindDir("tests") + s, err := newServerWithConfig(t, func(cfg *model.Config) { + testDir, _ := fileutils.FindDir("tests") - *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)) + *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") + }) require.NoError(t, err) err = s.Start() @@ -606,22 +620,16 @@ func TestSentry(t *testing.T) { require.NoError(t, err) SentryDSN = dsn.String() - configStore, _ := config.NewMemoryStore() - store, _ := config.NewStoreFromBacking(configStore, nil, false) - cfg := store.Get() - *cfg.ServiceSettings.ListenAddress = ":0" - *cfg.LogSettings.EnableSentry = false - *cfg.ServiceSettings.ConnectionSecurity = "TLS" - *cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem") - *cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem") - *cfg.LogSettings.EnableDiagnostics = true - - store.Set(cfg) - - s, err := NewServer(ConfigStore(store)) + s, err := newServerWithConfig(t, func(cfg *model.Config) { + *cfg.ServiceSettings.ListenAddress = ":0" + *cfg.LogSettings.EnableSentry = false + *cfg.ServiceSettings.ConnectionSecurity = "TLS" + *cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem") + *cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem") + *cfg.LogSettings.EnableDiagnostics = true + }) require.NoError(t, err) - // Route for just panicing s.Router.HandleFunc("/panic", func(writer http.ResponseWriter, request *http.Request) { panic("log this panic") }) @@ -656,19 +664,14 @@ func TestSentry(t *testing.T) { require.NoError(t, err) SentryDSN = dsn.String() - configStore, _ := config.NewMemoryStore() - store, _ := config.NewStoreFromBacking(configStore, nil, false) - cfg := store.Get() - *cfg.ServiceSettings.ListenAddress = ":0" - *cfg.ServiceSettings.ConnectionSecurity = "TLS" - *cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem") - *cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem") - *cfg.LogSettings.EnableSentry = true - *cfg.LogSettings.EnableDiagnostics = true - - store.Set(cfg) - - s, err := NewServer(ConfigStore(store)) + s, err := newServerWithConfig(t, func(cfg *model.Config) { + *cfg.ServiceSettings.ListenAddress = ":0" + *cfg.ServiceSettings.ConnectionSecurity = "TLS" + *cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem") + *cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem") + *cfg.LogSettings.EnableSentry = true + *cfg.LogSettings.EnableDiagnostics = true + }) require.NoError(t, err) // Route for just panicing