From c0f1cbf7271ff59795316c62006cfe6ad39c4b73 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Sat, 7 Jun 2025 17:35:08 -0400 Subject: [PATCH] [MM-64296] Help for AI: Respect env overrides for consoleLevel (#31278) * respect env overrides for consoleLevel; add tests * clean up test * merge conflict * improve parallelizability * be better commenting * better name for fn, comment explaining why we're using it * empty * empty * empty --------- Co-authored-by: Mattermost Build --- server/channels/api4/apitestlib.go | 22 ++++++-- server/channels/api4/apitestlib_test.go | 70 +++++++++++++++++++++++++ server/channels/utils/subpath.go | 4 +- 3 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 server/channels/api4/apitestlib_test.go diff --git a/server/channels/api4/apitestlib.go b/server/channels/api4/apitestlib.go index a13196a6db..22b238d764 100644 --- a/server/channels/api4/apitestlib.go +++ b/server/channels/api4/apitestlib.go @@ -99,7 +99,13 @@ func setupTestHelper(tb testing.TB, dbStore store.Store, sqlSettings *model.SqlS *memoryConfig.ServiceSettings.EnableLocalMode = true *memoryConfig.ServiceSettings.LocalModeSocketLocation = filepath.Join(tempWorkspace, "mattermost_local.sock") *memoryConfig.LogSettings.EnableSentry = false // disable error reporting during tests - *memoryConfig.LogSettings.ConsoleLevel = mlog.LvlStdLog.Name + + // Check for environment variable override for console log level (useful for debugging tests) + consoleLevel := os.Getenv("MM_LOGSETTINGS_CONSOLELEVEL") + if consoleLevel == "" { + consoleLevel = mlog.LvlStdLog.Name + } + *memoryConfig.LogSettings.ConsoleLevel = consoleLevel *memoryConfig.LogSettings.FileLocation = filepath.Join(tempWorkspace, "logs", "mattermost.log") *memoryConfig.AnnouncementSettings.AdminNoticesEnabled = false *memoryConfig.AnnouncementSettings.UserNoticesEnabled = false @@ -270,8 +276,13 @@ func SetupEnterprise(tb testing.TB, options ...app.Option) *TestHelper { tb.SkipNow() } + removeSpuriousErrors := func(config *model.Config) { + // If not set, you will receive an unactionable error in the console + *config.ServiceSettings.SiteURL = "http://localhost:8065" + } + dbStore, dbSettings, searchEngine := setupStores(tb) - th := setupTestHelper(tb, dbStore, dbSettings, searchEngine, true, true, nil, options) + th := setupTestHelper(tb, dbStore, dbSettings, searchEngine, true, true, removeSpuriousErrors, options) th.InitLogin(tb) return th @@ -355,7 +366,12 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { } func SetupEnterpriseWithStoreMock(tb testing.TB, options ...app.Option) *TestHelper { - th := setupTestHelper(tb, testlib.GetMockStoreForSetupFunctions(), nil, nil, true, false, nil, options) + removeSpuriousErrors := func(config *model.Config) { + // If not set, you will receive an unactionable error in the console + *config.ServiceSettings.SiteURL = "http://localhost:8065" + } + + th := setupTestHelper(tb, testlib.GetMockStoreForSetupFunctions(), nil, nil, true, false, removeSpuriousErrors, options) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) diff --git a/server/channels/api4/apitestlib_test.go b/server/channels/api4/apitestlib_test.go new file mode 100644 index 0000000000..a7d69f0387 --- /dev/null +++ b/server/channels/api4/apitestlib_test.go @@ -0,0 +1,70 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package api4 + +import ( + "os" + "testing" + + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/stretchr/testify/assert" +) + +func TestEnvironmentVariableHandling(t *testing.T) { + // TestEnvironmentVariableHandling should NEVER be run with t.Parallel() + + originalConsoleLevel := os.Getenv("MM_LOGSETTINGS_CONSOLELEVEL") + defer func() { + // Restore original environment variables + if originalConsoleLevel != "" { + os.Setenv("MM_LOGSETTINGS_CONSOLELEVEL", originalConsoleLevel) + } else { + os.Unsetenv("MM_LOGSETTINGS_CONSOLELEVEL") + } + }() + + t.Run("MM_LOGSETTINGS_CONSOLELEVEL should be respected when set", func(t *testing.T) { + // never run with t.Parallel() + + // Set the console level environment variable + os.Setenv("MM_LOGSETTINGS_CONSOLELEVEL", "ERROR") + defer os.Unsetenv("MM_LOGSETTINGS_CONSOLELEVEL") + + th := SetupEnterprise(t) + defer th.TearDown() + + // Verify the console level was set from the environment variable + config := th.App.Config() + assert.Equal(t, "ERROR", *config.LogSettings.ConsoleLevel) + }) + + t.Run("Only MM_LOGSETTINGS_CONSOLELEVEL is manually processed", func(t *testing.T) { + // never run with t.Parallel() + + // This test verifies that we haven't accidentally enabled general environment + // variable processing - we only manually handle MM_LOGSETTINGS_CONSOLELEVEL + + // First, test without MM_LOGSETTINGS_CONSOLELEVEL set + os.Unsetenv("MM_LOGSETTINGS_CONSOLELEVEL") + + th1 := SetupEnterprise(t) + config1 := th1.App.Config() + defaultConsoleLevel := *config1.LogSettings.ConsoleLevel + th1.TearDown() + + // Now test with MM_LOGSETTINGS_CONSOLELEVEL set + os.Setenv("MM_LOGSETTINGS_CONSOLELEVEL", "DEBUG") + defer os.Unsetenv("MM_LOGSETTINGS_CONSOLELEVEL") + + th2 := SetupEnterprise(t) + config2 := th2.App.Config() + customConsoleLevel := *config2.LogSettings.ConsoleLevel + th2.TearDown() + + // Verify our manual implementation works + assert.Equal(t, mlog.LvlStdLog.Name, defaultConsoleLevel, "Default should be stdlog") + assert.Equal(t, "DEBUG", customConsoleLevel, "Environment variable should be respected") + assert.NotEqual(t, defaultConsoleLevel, customConsoleLevel, "Values should be different") + }) +} diff --git a/server/channels/utils/subpath.go b/server/channels/utils/subpath.go index b2a293593e..9072bc6512 100644 --- a/server/channels/utils/subpath.go +++ b/server/channels/utils/subpath.go @@ -182,8 +182,8 @@ func UpdateAssetsSubpathFromConfig(config *model.Config) error { return nil } - // Similarly, don't rewrite during a CI build, when the assets may not even be present. - if os.Getenv("IS_CI") == "true" { + // Similarly, don't rewrite during a CI build, or a local testing build, when the assets may not even be present. + if os.Getenv("IS_CI") == "true" || os.Getenv("IS_LOCAL_TESTING") == "true" { mlog.Debug("Skipping update to assets subpath since CI build") return nil }