From 6179675c71a69aa448c2912a36367ee1b06400f7 Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 20 Mar 2026 02:41:32 +0530 Subject: [PATCH 01/11] Moved configuration accessors to a new file structure under `config/` --- browser/src/app.ts | 4 +-- .../accessors.ts} | 19 +++++++---- .../config_helpers.ts => config/loader.ts} | 33 +++++++------------ .../src/e2e/post_and_scroll_scenario.spec.ts | 4 +-- .../log.test.ts => logger/index.test.ts} | 4 +-- browser/src/{utils/log.ts => logger/index.ts} | 2 +- browser/src/routes/browser.test.ts | 2 +- browser/src/routes/browser.ts | 2 +- browser/src/services/browser_manager.ts | 4 +-- browser/src/smoke_simulations/index.ts | 2 +- 10 files changed, 35 insertions(+), 41 deletions(-) rename browser/src/{utils/config_accessors.ts => config/accessors.ts} (60%) rename browser/src/{utils/config_helpers.ts => config/loader.ts} (80%) rename browser/src/{utils/log.test.ts => logger/index.test.ts} (98%) rename browser/src/{utils/log.ts => logger/index.ts} (99%) diff --git a/browser/src/app.ts b/browser/src/app.ts index 8d05939bd..ffdb4dad5 100644 --- a/browser/src/app.ts +++ b/browser/src/app.ts @@ -8,8 +8,8 @@ import {type Ajv} from 'ajv'; import browserRoutes from './routes/browser.js'; import healthRoutes from './routes/health.js'; -import {isConsoleLoggingEnabled} from './utils/config_accessors.js'; -import {getServerLoggerConfig, createLogger} from './utils/log.js'; +import {isConsoleLoggingEnabled} from './config/accessors.js'; +import {getServerLoggerConfig, createLogger} from './logger/index.js'; export async function applyMiddleware(fastifyInstance: FastifyInstance) { const baseSchema = { diff --git a/browser/src/utils/config_accessors.ts b/browser/src/config/accessors.ts similarity index 60% rename from browser/src/utils/config_accessors.ts rename to browser/src/config/accessors.ts index adb14fd3e..80ac9e70e 100644 --- a/browser/src/utils/config_accessors.ts +++ b/browser/src/config/accessors.ts @@ -1,30 +1,35 @@ // Copyright (c) 2019-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {configJson, browserControllerConfigJson} from './config_helpers.js'; +import {browserControllerConfigJson} from './loader.js'; +/** + * Server URl is always passed as a parameter to the browser controller while + * its created. So we don't need to read it from the config.json. But we need + * hardcoded value for tests and smoke simulations. + */ export function getMattermostServerURL(): string { - return configJson.ConnectionConfiguration.ServerURL; + return 'http://localhost:8065'; } export function isConsoleLoggingEnabled(): boolean { - return configJson.BrowserLogSettings.EnableConsole; + return browserControllerConfigJson.LogSettings.EnableConsole; } export function getConsoleLoggingLevel(): string { - return configJson.BrowserLogSettings.ConsoleLevel; + return browserControllerConfigJson.LogSettings.ConsoleLevel; } export function isFileLoggingEnabled(): boolean { - return configJson.BrowserLogSettings.EnableFile; + return browserControllerConfigJson.LogSettings.EnableFile; } export function getFileLoggingLevel(): string { - return configJson.BrowserLogSettings.FileLevel; + return browserControllerConfigJson.LogSettings.FileLevel; } export function getFileLoggingLocation(): string { - return configJson.BrowserLogSettings.FileLocation; + return browserControllerConfigJson.LogSettings.FileLocation; } export function isBrowserHeadless(): boolean { diff --git a/browser/src/utils/config_helpers.ts b/browser/src/config/loader.ts similarity index 80% rename from browser/src/utils/config_helpers.ts rename to browser/src/config/loader.ts index eb361df94..63936c302 100644 --- a/browser/src/utils/config_helpers.ts +++ b/browser/src/config/loader.ts @@ -10,32 +10,26 @@ import {z as zod} from 'zod'; const logLabelLevels = Object.values(pino.levels.labels); -const SliceOfConfigJsonSchema = zod.object({ - ConnectionConfiguration: zod.object({ - ServerURL: zod.string().min(1, 'ConnectionConfiguration.ServerURL cannot be empty'), - }), - BrowserLogSettings: zod.object({ +const BrowserControllerConfigJsonSchema = zod.object({ + SimulationId: zod.string().min(1, 'SimulationId cannot be empty'), + RunInHeadless: zod.boolean(), + SimulationTimeoutMs: zod + .number() + .gte(0, 'SimulationTimeoutMs must be greater than or equal to 0. Set to 0 to disable timeout.'), + EnabledPlugins: zod.boolean(), + LogSettings: zod.object({ EnableConsole: zod.boolean(), ConsoleLevel: zod.enum(logLabelLevels, { - message: `BrowserLogSettings.ConsoleLevel must be one of: ${logLabelLevels.join(', ')}`, + message: `Browser LogSettings.ConsoleLevel must be one of: ${logLabelLevels.join(', ')}`, }), EnableFile: zod.boolean(), FileLevel: zod.enum(logLabelLevels, { - message: `BrowserLogSettings.FileLevel must be one of: ${logLabelLevels.join(', ')}`, + message: `Browser LogSettings.FileLevel must be one of: ${logLabelLevels.join(', ')}`, }), - FileLocation: zod.string().min(1, 'BrowserLogSettings.FileLocation cannot be empty'), + FileLocation: zod.string().min(1, 'Browser LogSettings.FileLocation cannot be empty'), }), }); -const BrowserControllerConfigJsonSchema = zod.object({ - RunInHeadless: zod.boolean(), - SimulationTimeoutMs: zod - .number() - .gte(0, 'SimulationTimeoutMs must be greater than or equal to 0. Set to 0 to disable timeout.'), - EnabledPlugins: zod.boolean(), - SimulationId: zod.string().min(1, 'SimulationId cannot be empty'), -}); - const GoModFileName = 'go.mod'; const GitFolderName = '.git'; const BinFolderName = 'bin'; @@ -89,11 +83,6 @@ function loadJsonFile(filePath: string, schema: zod.ZodSchema): T { } const ConfigFolderName = 'config'; - -const ConfigFileName = 'config.json'; -const configJsonPath = join(getRootDirectory(), ConfigFolderName, ConfigFileName); -export const configJson = loadJsonFile(configJsonPath, SliceOfConfigJsonSchema); - const BrowserControllerConfigFileName = 'browsercontroller.json'; const browserControllerConfigJsonPath = join(getRootDirectory(), ConfigFolderName, BrowserControllerConfigFileName); export const browserControllerConfigJson = loadJsonFile( diff --git a/browser/src/e2e/post_and_scroll_scenario.spec.ts b/browser/src/e2e/post_and_scroll_scenario.spec.ts index c1102a59f..4a9241dca 100644 --- a/browser/src/e2e/post_and_scroll_scenario.spec.ts +++ b/browser/src/e2e/post_and_scroll_scenario.spec.ts @@ -6,8 +6,8 @@ import {test} from '@playwright/test'; import {type BrowserInstance} from '@mattermost/loadtest-browser-lib'; import {postAndScrollScenario} from '../simulations/post_and_scroll_scenario.js'; -import {getMattermostServerURL} from '../utils/config_accessors.js'; -import {createNullLogger} from '../utils/log.js'; +import {getMattermostServerURL} from '../config/accessors.js'; +import {createNullLogger} from '../logger/index.js'; test('Post and Scroll Scenario', async ({page}) => { const browserInstance = { diff --git a/browser/src/utils/log.test.ts b/browser/src/logger/index.test.ts similarity index 98% rename from browser/src/utils/log.test.ts rename to browser/src/logger/index.test.ts index d2593d15c..72a6b5393 100644 --- a/browser/src/utils/log.test.ts +++ b/browser/src/logger/index.test.ts @@ -79,8 +79,8 @@ import { isFileLoggingEnabled, getFileLoggingLevel, getFileLoggingLocation, -} from './config_accessors.js'; -import {createLogger, getServerLoggerConfig} from './log.js'; +} from '../config/accessors.js'; +import {createLogger, getServerLoggerConfig} from './index.js'; describe('createLogger', () => { const mockLogger = { diff --git a/browser/src/utils/log.ts b/browser/src/logger/index.ts similarity index 99% rename from browser/src/utils/log.ts rename to browser/src/logger/index.ts index 7da90e65e..97abd430a 100644 --- a/browser/src/utils/log.ts +++ b/browser/src/logger/index.ts @@ -21,7 +21,7 @@ import { isFileLoggingEnabled, getFileLoggingLevel, getFileLoggingLocation, -} from './config_accessors.js'; +} from '../config/accessors.js'; const pino = require('pino'); diff --git a/browser/src/routes/browser.test.ts b/browser/src/routes/browser.test.ts index 1cb62b584..9ab290a9b 100644 --- a/browser/src/routes/browser.test.ts +++ b/browser/src/routes/browser.test.ts @@ -154,7 +154,7 @@ describe('API /browsers', () => { test('should return 400 when server URL is missing in config', async () => { // Mock getMattermostServerURL to return empty string - const {getMattermostServerURL} = await import('../utils/config_accessors.js'); + const {getMattermostServerURL} = await import('../config/accessors.js'); vi.mocked(getMattermostServerURL).mockReturnValueOnce(''); await app.listen({port}); diff --git a/browser/src/routes/browser.ts b/browser/src/routes/browser.ts index d68168579..57194b374 100644 --- a/browser/src/routes/browser.ts +++ b/browser/src/routes/browser.ts @@ -7,7 +7,7 @@ import {postSchema, deleteSchema, getSchema} from './browser.schema.js'; import type {IReply} from './types.js'; import {browserTestSessionManager} from '../services/browser_manager.js'; -import {getSimulationId, isBrowserHeadless} from '../utils/config_accessors.js'; +import {getSimulationId, isBrowserHeadless} from '../config/accessors.js'; export default async function browserRoutes(fastify: FastifyInstance) { // Register shutdown hook when routes are loaded diff --git a/browser/src/services/browser_manager.ts b/browser/src/services/browser_manager.ts index c2e9f5988..4d8d543d9 100644 --- a/browser/src/services/browser_manager.ts +++ b/browser/src/services/browser_manager.ts @@ -9,8 +9,8 @@ import {type BrowserInstance} from '@mattermost/loadtest-browser-lib'; import {testManager} from './test_manager.js'; import {log} from '../app.js'; -import {getSimulationTimeoutMs} from '../utils/config_accessors.js'; -import {screenshotsDirectory} from '../utils/config_helpers.js'; +import {getSimulationTimeoutMs} from '../config/accessors.js'; +import {screenshotsDirectory} from '../config/loader.js'; const CLEANUP_TIMEOUT_MS = 4_000; diff --git a/browser/src/smoke_simulations/index.ts b/browser/src/smoke_simulations/index.ts index 41d4165fb..5afe95877 100644 --- a/browser/src/smoke_simulations/index.ts +++ b/browser/src/smoke_simulations/index.ts @@ -9,7 +9,7 @@ import ms from 'ms'; import {browserTestSessionManager} from '../services/browser_manager.js'; import {SimulationsRegistry} from '../registry.js'; -import {getMattermostServerURL} from '../utils/config_accessors.js'; +import {getMattermostServerURL} from '../config/accessors.js'; interface SmokeSimulationConfig { users: Array<{username: string; password: string}>; From ab46bb1c0bbcad0cb7382b86c052b2c6be7e6add Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 20 Mar 2026 02:58:09 +0530 Subject: [PATCH 02/11] Refactor logger tests to use updated config accessors path and mock node modules correctly --- browser/src/logger/index.test.ts | 26 +++----------------------- browser/src/routes/browser.test.ts | 9 ++------- 2 files changed, 5 insertions(+), 30 deletions(-) diff --git a/browser/src/logger/index.test.ts b/browser/src/logger/index.test.ts index 72a6b5393..a84261671 100644 --- a/browser/src/logger/index.test.ts +++ b/browser/src/logger/index.test.ts @@ -5,7 +5,7 @@ import type {FastifyBaseLogger} from 'fastify'; import {vi, describe, it, expect, beforeEach} from 'vitest'; // Mock all the dependencies with simple implementations -vi.mock('./config_accessors.js', () => ({ +vi.mock('../config/accessors.js', () => ({ isConsoleLoggingEnabled: vi.fn(() => false), getConsoleLoggingLevel: vi.fn(() => 'info'), isFileLoggingEnabled: vi.fn(() => false), @@ -13,27 +13,7 @@ vi.mock('./config_accessors.js', () => ({ getFileLoggingLocation: vi.fn(() => 'logs/browser.log'), })); -vi.mock('./config_helpers.js', () => ({ - configJson: { - ConnectionConfiguration: {ServerURL: 'http://localhost:8065'}, - BrowserLogSettings: { - EnableConsole: false, - ConsoleLevel: 'info', - EnableFile: false, - FileLevel: 'debug', - FileLocation: 'logs/browser.log', - }, - }, - browserControllerConfigJson: { - RunInHeadless: true, - SimulationTimeoutMs: 60000, - SimulationId: 'postAndScroll', - }, - getRootDirectory: vi.fn(() => '/mock/root'), - screenshotsDirectory: '/mock/root/browser/screenshots', -})); - -vi.mock('path', async (importOriginal) => { +vi.mock('node:path', async (importOriginal) => { const actual = await importOriginal(); return { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -44,7 +24,7 @@ vi.mock('path', async (importOriginal) => { }; }); -vi.mock('url', () => ({ +vi.mock('node:url', () => ({ fileURLToPath: vi.fn(() => '/mock/path/to/file.js'), })); diff --git a/browser/src/routes/browser.test.ts b/browser/src/routes/browser.test.ts index 9ab290a9b..a083bd7f6 100644 --- a/browser/src/routes/browser.test.ts +++ b/browser/src/routes/browser.test.ts @@ -7,12 +7,11 @@ import {describe, expect, test, beforeEach, afterEach, vi} from 'vitest'; import {createApp} from '../app.js'; -vi.mock('../utils/config_accessors.js', async (importOriginal) => { +vi.mock('../config/accessors.js', async (importOriginal) => { const actual = await importOriginal(); return { // eslint-disable-next-line @typescript-eslint/no-explicit-any ...(actual as any), - getMattermostServerURL: vi.fn().mockReturnValue('http://localhost:8065'), isBrowserHeadless: vi.fn().mockReturnValue(true), getSimulationId: vi.fn().mockReturnValue('postAndScroll'), }; @@ -152,11 +151,7 @@ describe('API /browsers', () => { }); }); - test('should return 400 when server URL is missing in config', async () => { - // Mock getMattermostServerURL to return empty string - const {getMattermostServerURL} = await import('../config/accessors.js'); - vi.mocked(getMattermostServerURL).mockReturnValueOnce(''); - + test('should return 400 when server URL is missing in request body', async () => { await app.listen({port}); const response = await supertest(`http://localhost:${port}`) From eb84dc28ea1ac0fef9118843bea75d11f736c1af Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 20 Mar 2026 03:00:32 +0530 Subject: [PATCH 03/11] Moved LogSettings in browsercontroller.json to manage console and file logging from config.json --- config/browsercontroller.sample.json | 9 +++- config/config.sample.json | 7 --- config/config.sample.toml | 7 --- docs/config/browsercontroller.md | 50 ++++++++++++++++++++ docs/config/config.md | 50 -------------------- examples/config/perfcomp/json/config.json | 4 -- examples/config/release/json/config.json | 4 -- loadtest/config.go | 11 ----- loadtest/control/browsercontroller/config.go | 44 +++++++++++++++++ loadtest/loadtest_test.go | 4 -- loadtest/user/userentity/helper_test.go | 9 +--- 11 files changed, 103 insertions(+), 96 deletions(-) create mode 100644 loadtest/control/browsercontroller/config.go diff --git a/config/browsercontroller.sample.json b/config/browsercontroller.sample.json index 7765cfc3e..1c36aa292 100644 --- a/config/browsercontroller.sample.json +++ b/config/browsercontroller.sample.json @@ -2,5 +2,12 @@ "SimulationId": "mattermostPostAndScroll", "RunInHeadless": true, "SimulationTimeoutMs": 60000, - "EnabledPlugins": false + "EnabledPlugins": false, + "LogSettings": { + "EnableConsole": true, + "ConsoleLevel": "debug", + "EnableFile": true, + "FileLevel": "debug", + "FileLocation": "browseragent.log" + } } diff --git a/config/config.sample.json b/config/config.sample.json index 34f24bb06..9e1202280 100644 --- a/config/config.sample.json +++ b/config/config.sample.json @@ -62,12 +62,5 @@ "FileJson": true, "FileLocation": "ltagent.log", "EnableColor": false - }, - "BrowserLogSettings": { - "EnableConsole": true, - "ConsoleLevel": "debug", - "EnableFile": true, - "FileLevel": "debug", - "FileLocation": "browseragent.log" } } diff --git a/config/config.sample.toml b/config/config.sample.toml index 3e8dc13e2..63c33f360 100644 --- a/config/config.sample.toml +++ b/config/config.sample.toml @@ -28,13 +28,6 @@ FileJson = true FileLevel = 'INFO' FileLocation = 'ltagent.log' -[BrowserLogSettings] -EnableConsole = true -ConsoleLevel = 'debug' -EnableFile = true -FileLevel = 'debug' -FileLocation = 'browseragent.log' - [UserControllerConfiguration] ServerVersion = '' Type = 'simulative' diff --git a/docs/config/browsercontroller.md b/docs/config/browsercontroller.md index 362200e65..eecc97b60 100644 --- a/docs/config/browsercontroller.md +++ b/docs/config/browsercontroller.md @@ -37,3 +37,53 @@ When set to `false`, the load test uses only the predefined simulations built in See [Plugin Browser Load Testing](../plugin_browser_loadtest.md) for more details. **Default:** `false` + +## LogSettings + +### EnableConsole + +*bool* + +When true, the browser server outputs log messages to the console based on ConsoleLevel option. + +### ConsoleLevel + +*string* + +Level of detail at which log events are written to the console. + +Possible values (in order of decreasing verbosity, these are case sensitive): +- `trace` +- `debug` +- `info` +- `warn` +- `error` +- `fatal` + +### EnableFile + +*bool* + +When true, the browser server outputs log messages to the file specified by the `FileLocation` setting. + +### FileLevel + +*string* + +Level of detail at which log events are written to log files. Exactly same as `ConsoleLevel` as mentioned above. + +Possible values (in order of decreasing verbosity, these are case sensitive): +- `trace` +- `debug` +- `info` +- `warn` +- `error` +- `fatal` + +When both `EnableConsole` and `EnableFile` are true, the logs are written asynchronously to reduce overhead. + +### FileLocation + +*string* + +The location of the log file. Must be a valid file path including the file name. diff --git a/docs/config/config.md b/docs/config/config.md index c24be77be..6083b88e7 100644 --- a/docs/config/config.md +++ b/docs/config/config.md @@ -183,53 +183,3 @@ The location of the log file. *bool* When true enables colored output. - -## BrowserLogSettings - -### EnableConsole - -*bool* - -When true, the browser server outputs log messages to the console based on ConsoleLevel option. - -### ConsoleLevel - -*string* - -Level of detail at which log events are written to the console. - -Possible values (in order of decreasing verbosity, these are case sensitive): -- `trace` -- `debug` -- `info` -- `warn` -- `error` -- `fatal` - -### EnableFile - -*bool* - -When true, the browser server outputs log messages to the file specified by the `FileLocation` setting. - -### FileLevel - -*string* - -Level of detail at which log events are written to log files. Exactly same as `ConsoleLevel` as mentioned above. - -Possible values (in order of decreasing verbosity, these are case sensitive): -- `trace` -- `debug` -- `info` -- `warn` -- `error` -- `fatal` - -When both `EnableConsole` and `EnableFile` are true, the logs are written asynchronously to reduce overhead. - -### FileLocation - -*string* - -The location of the log file. diff --git a/examples/config/perfcomp/json/config.json b/examples/config/perfcomp/json/config.json index 25da18e7c..78b94797e 100644 --- a/examples/config/perfcomp/json/config.json +++ b/examples/config/perfcomp/json/config.json @@ -1,8 +1,4 @@ { - "BrowserLogSettings": { - "ConsoleLevel": "debug", - "EnableConsole": true - }, "InstanceConfiguration": { "NumChannels": 0, "PercentDirectChannels": 0, diff --git a/examples/config/release/json/config.json b/examples/config/release/json/config.json index 481699ef3..988bde208 100644 --- a/examples/config/release/json/config.json +++ b/examples/config/release/json/config.json @@ -1,8 +1,4 @@ { - "BrowserLogSettings": { - "ConsoleLevel": "debug", - "EnableConsole": true - }, "InstanceConfiguration": { "NumChannels": 0, "PercentDirectChannels": 0, diff --git a/loadtest/config.go b/loadtest/config.go index 4ae3e035c..46e826cab 100644 --- a/loadtest/config.go +++ b/loadtest/config.go @@ -141,16 +141,6 @@ type UsersConfiguration struct { PercentOfUsersAreAdmin float64 `default:"0.0005" validate:"range:[0,1]"` } -// BrowserLogSettings holds information to be used to initialize the logger for the LTBrowser API -// refer to /browser/src/utils/log.ts -type BrowserLogSettings struct { - EnableConsole bool `default:"false"` - ConsoleLevel string `default:"error" validate:"oneof:{trace, debug, info, warn, error, fatal}"` - EnableFile bool `default:"true"` - FileLevel string `default:"debug" validate:"oneof:{trace, debug, info, warn, error, fatal}"` - FileLocation string `default:"browseragent.log"` -} - // Config holds information needed to create and initialize a new load-test // agent. type Config struct { @@ -159,7 +149,6 @@ type Config struct { InstanceConfiguration InstanceConfiguration UsersConfiguration UsersConfiguration LogSettings logger.Settings - BrowserLogSettings BrowserLogSettings } // IsValid reports whether a given Config is valid or not. diff --git a/loadtest/control/browsercontroller/config.go b/loadtest/control/browsercontroller/config.go new file mode 100644 index 000000000..a4bbf83c6 --- /dev/null +++ b/loadtest/control/browsercontroller/config.go @@ -0,0 +1,44 @@ +// Copyright (c) 2019-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package browsercontroller + +import ( + "github.com/mattermost/mattermost-load-test-ng/defaults" +) + +// BrowserLogSettings holds information to be used to initialize the logger for the LTBrowser API +// refer to /browser/src/utils/log.ts +type BrowserLogSettings struct { + EnableConsole bool `default:"false"` + ConsoleLevel string `default:"error" validate:"oneof:{trace, debug, info, warn, error, fatal}"` + EnableFile bool `default:"true"` + FileLevel string `default:"debug" validate:"oneof:{trace, debug, info, warn, error, fatal}"` + FileLocation string `default:"browseragent.log"` +} + +// Config holds information needed to run a BrowserController. +type Config struct { + // The ID of the simulation to run. + SimulationId string `default:"mattermostPostAndScroll" validate:"notempty"` + // Whether to run the browser in headless mode. + RunInHeadless bool `default:"true"` + // The timeout in milliseconds for browser simulations. + SimulationTimeoutMs int `default:"60000" validate:"range:[0,]"` + // Whether to enable plugins in the browser simulation. + EnabledPlugins bool `default:"false"` + // Log settings for the LTBrowser API + LogSettings BrowserLogSettings +} + +// ReadConfig reads the configuration file from the given string. If the string +// is empty, it will return a config with default values. +func ReadConfig(configFilePath string) (*Config, error) { + var cfg Config + + if err := defaults.ReadFrom(configFilePath, "./config/browsercontroller.json", &cfg); err != nil { + return nil, err + } + + return &cfg, nil +} diff --git a/loadtest/loadtest_test.go b/loadtest/loadtest_test.go index 0145758df..ae368cd10 100644 --- a/loadtest/loadtest_test.go +++ b/loadtest/loadtest_test.go @@ -50,10 +50,6 @@ var ltConfig = Config{ ConsoleLevel: "ERROR", FileLevel: "ERROR", }, - BrowserLogSettings: BrowserLogSettings{ - ConsoleLevel: "error", - FileLevel: "error", - }, } func newController(id int, status chan<- control.UserStatus) (control.UserController, error) { diff --git a/loadtest/user/userentity/helper_test.go b/loadtest/user/userentity/helper_test.go index 7c09e722e..992d306d5 100644 --- a/loadtest/user/userentity/helper_test.go +++ b/loadtest/user/userentity/helper_test.go @@ -53,14 +53,7 @@ type config struct { AvgSessionsPerUser int `default:"1" validate:"range:[1,]"` PercentOfUsersAreAdmin float64 `default:"0.0005" validate:"range:[0,1]"` } - LogSettings logger.Settings - BrowserLogSettings struct { - EnableConsole bool `default:"false"` - ConsoleLevel string `default:"error" validate:"oneof:{trace, debug, info, warn, error, fatal}"` - EnableFile bool `default:"true"` - FileLevel string `default:"error" validate:"oneof:{trace, debug, info, warn, error, fatal}"` - FileLocation string `default:"browseragent.log"` - } + LogSettings logger.Settings } type TestHelper struct { From c9951f52a9d697f4d282f3f10ab32ee35e5eede2 Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 20 Mar 2026 03:15:51 +0530 Subject: [PATCH 04/11] Update default logging settings in BrowserLogSettings to enable console logging and set console log level to debug --- loadtest/control/browsercontroller/config.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/loadtest/control/browsercontroller/config.go b/loadtest/control/browsercontroller/config.go index a4bbf83c6..48255b200 100644 --- a/loadtest/control/browsercontroller/config.go +++ b/loadtest/control/browsercontroller/config.go @@ -10,8 +10,8 @@ import ( // BrowserLogSettings holds information to be used to initialize the logger for the LTBrowser API // refer to /browser/src/utils/log.ts type BrowserLogSettings struct { - EnableConsole bool `default:"false"` - ConsoleLevel string `default:"error" validate:"oneof:{trace, debug, info, warn, error, fatal}"` + EnableConsole bool `default:"true"` + ConsoleLevel string `default:"debug" validate:"oneof:{trace, debug, info, warn, error, fatal}"` EnableFile bool `default:"true"` FileLevel string `default:"debug" validate:"oneof:{trace, debug, info, warn, error, fatal}"` FileLocation string `default:"browseragent.log"` @@ -21,12 +21,16 @@ type BrowserLogSettings struct { type Config struct { // The ID of the simulation to run. SimulationId string `default:"mattermostPostAndScroll" validate:"notempty"` + // Whether to run the browser in headless mode. RunInHeadless bool `default:"true"` + // The timeout in milliseconds for browser simulations. SimulationTimeoutMs int `default:"60000" validate:"range:[0,]"` + // Whether to enable plugins in the browser simulation. EnabledPlugins bool `default:"false"` + // Log settings for the LTBrowser API LogSettings BrowserLogSettings } From ab4045314296c5c533072685f8bdcb4a70db1099 Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 20 Mar 2026 05:11:11 +0530 Subject: [PATCH 05/11] Add browser controller configuration validation and setup for agent tests - Introduced a new `setupAgentType` function to streamline the setup of agent types in tests. - Updated `TestIsBrowserAgentInstance` and `TestBrowserAgentConfigValidation` to utilize the new setup function. - Enhanced the `createLoadAgentHandler` to validate the `browsercontroller.json` configuration before creating browser agents. - Added logic in the Terraform agent configuration to upload the `browsercontroller.json` to browser agent instances, ensuring valid configurations are used during load tests. --- api/agent.go | 19 +++++ api/agent_test.go | 149 ++++++++++++++++++++++++++-------- deployment/terraform/agent.go | 26 ++++++ 3 files changed, 161 insertions(+), 33 deletions(-) diff --git a/api/agent.go b/api/agent.go index e0fd1f01c..168b717d1 100644 --- a/api/agent.go +++ b/api/agent.go @@ -122,6 +122,25 @@ func (a *api) createLoadAgentHandler(w http.ResponseWriter, r *http.Request) { mlog.Warn("failed to detect agent_type. Going ahead assuming it's a server agent", mlog.Err(err)) } + // Read and validate the browsercontroller.json that was uploaded by + // Terraform to confirm it landed correctly and contains valid values + // before proceeding with browser agent creation if it's a browser agent instance. + if isBAInstance { + bccfg, err := browsercontroller.ReadConfig("./config/browsercontroller.json") + if err != nil { + writeAgentResponse(w, http.StatusBadRequest, &client.AgentResponse{ + Error: fmt.Sprintf("could not read browser controller config: %s", err), + }) + return + } + if err := defaults.Validate(bccfg); err != nil { + writeAgentResponse(w, http.StatusBadRequest, &client.AgentResponse{ + Error: fmt.Sprintf("could not validate browser controller config: %s", err), + }) + return + } + } + newC, err := NewControllerWrapper(<Config, ucConfig, 0, agentId, a.metrics, isBAInstance) if err != nil { writeAgentResponse(w, http.StatusBadRequest, &client.AgentResponse{ diff --git a/api/agent_test.go b/api/agent_test.go index b359277b5..ac6b08365 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -32,7 +32,23 @@ type requestData struct { SimulControllerConfig *simulcontroller.Config `json:",omitempty"` } +func setupAgentType(t *testing.T, agentType string) { + t.Helper() + originalHome := os.Getenv("HOME") + tempDir, err := os.MkdirTemp("", "test_home_"+agentType) + require.NoError(t, err) + agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) + require.NoError(t, os.WriteFile(agentTypeFile, []byte(agentType), 0644)) + os.Setenv("HOME", tempDir) + t.Cleanup(func() { + os.Setenv("HOME", originalHome) + os.RemoveAll(tempDir) + }) +} + func TestAgentAPI(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -181,6 +197,8 @@ func TestAgentAPI(t *testing.T) { } func TestAgentAPIConcurrency(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -303,22 +321,8 @@ func TestGetUserCredentials(t *testing.T) { } func TestIsBrowserAgentInstance(t *testing.T) { - // Get original home directory to restore later - originalHome := os.Getenv("HOME") - t.Run("returns true when agent_type.txt contains browser_agent", func(t *testing.T) { - // Create temporary directory to use as home - tempDir, err := os.MkdirTemp("", "test_home_browser") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - // Set temporary home directory - os.Setenv("HOME", tempDir) - defer os.Setenv("HOME", originalHome) - - agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) - err = os.WriteFile(agentTypeFile, []byte(" "+deployment.AgentTypeBrowser+" \n"), 0644) - require.NoError(t, err) + setupAgentType(t, deployment.AgentTypeBrowser) result, err := isBrowserAgentInstance() require.NoError(t, err) @@ -326,16 +330,7 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt contains server_agent", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "test_home_server") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - os.Setenv("HOME", tempDir) - defer os.Setenv("HOME", originalHome) - - agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) - err = os.WriteFile(agentTypeFile, []byte(deployment.AgentTypeServer), 0644) - require.NoError(t, err) + setupAgentType(t, deployment.AgentTypeServer) result, err := isBrowserAgentInstance() require.NoError(t, err) @@ -343,12 +338,14 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt file does not exist", func(t *testing.T) { + originalHome := os.Getenv("HOME") tempDir, err := os.MkdirTemp("", "test_home_missing") require.NoError(t, err) - defer os.RemoveAll(tempDir) - + t.Cleanup(func() { + os.Setenv("HOME", originalHome) + os.RemoveAll(tempDir) + }) os.Setenv("HOME", tempDir) - defer os.Setenv("HOME", originalHome) result, err := isBrowserAgentInstance() require.Error(t, err) @@ -356,19 +353,105 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt contains unknown content", func(t *testing.T) { + originalHome := os.Getenv("HOME") tempDir, err := os.MkdirTemp("", "test_home_unknown") require.NoError(t, err) - defer os.RemoveAll(tempDir) - + t.Cleanup(func() { + os.Setenv("HOME", originalHome) + os.RemoveAll(tempDir) + }) os.Setenv("HOME", tempDir) - defer os.Setenv("HOME", originalHome) agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) - err = os.WriteFile(agentTypeFile, []byte("unknown_agent_type"), 0644) - require.NoError(t, err) + require.NoError(t, os.WriteFile(agentTypeFile, []byte("unknown_agent_type"), 0644)) result, err := isBrowserAgentInstance() require.Error(t, err) require.False(t, result) }) } + +func TestBrowserAgentConfigValidation(t *testing.T) { + handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) + server := httptest.NewServer(handler) + defer server.Close() + + mmServer := createFakeMMServer() + defer mmServer.Close() + + e := httpexpect.New(t, server.URL+"/loadagent") + + ltConfig := loadtest.Config{} + err := defaults.Set(<Config) + require.NoError(t, err) + ltConfig.UserControllerConfiguration.ServerVersion = control.MinSupportedVersion.String() + ltConfig.ConnectionConfiguration.ServerURL = mmServer.URL + ltConfig.UsersConfiguration.MaxActiveUsers = 100 + + t.Run("fails when browsercontroller.json is missing", func(t *testing.T) { + setupAgentType(t, deployment.AgentTypeBrowser) + + rd := requestData{LoadTestConfig: ltConfig} + e.POST("/create").WithQuery("id", "ltb0").WithJSON(rd). + Expect().Status(http.StatusBadRequest). + JSON().Object().ContainsKey("error") + }) + + t.Run("succeeds with valid browsercontroller.json", func(t *testing.T) { + setupAgentType(t, deployment.AgentTypeBrowser) + + // ReadConfig uses "./config/browsercontroller.json" relative to cwd, + // so we need to be at the repo root for the path to resolve. + originalDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir("..")) + t.Cleanup(func() { os.Chdir(originalDir) }) + + rd := requestData{LoadTestConfig: ltConfig} + obj := e.POST("/create").WithQuery("id", "ltb1").WithJSON(rd). + Expect().Status(http.StatusCreated). + JSON().Object().ValueEqual("id", "ltb1") + rawMsg := obj.Value("message").String().Raw() + require.Equal(t, "load-test agent created", rawMsg) + + e.POST("ltb1/stop").Expect().Status(http.StatusOK) + e.DELETE("ltb1").Expect().Status(http.StatusOK) + }) + + t.Run("fails with invalid browsercontroller.json values", func(t *testing.T) { + setupAgentType(t, deployment.AgentTypeBrowser) + + // Create a temporary directory with an invalid browsercontroller.json + tempDir, err := os.MkdirTemp("", "test_browser_invalid_config") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tempDir) }) + + configDir := filepath.Join(tempDir, "config") + require.NoError(t, os.MkdirAll(configDir, 0755)) + + invalidConfig := `{ + "SimulationId": "", + "RunInHeadless": true, + "SimulationTimeoutMs": -1, + "EnabledPlugins": false, + "LogSettings": { + "EnableConsole": true, + "ConsoleLevel": "invalid_level", + "EnableFile": true, + "FileLevel": "debug", + "FileLocation": "browseragent.log" + } + }` + require.NoError(t, os.WriteFile(filepath.Join(configDir, "browsercontroller.json"), []byte(invalidConfig), 0644)) + + originalDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(tempDir)) + t.Cleanup(func() { os.Chdir(originalDir) }) + + rd := requestData{LoadTestConfig: ltConfig} + e.POST("/create").WithQuery("id", "ltb2").WithJSON(rd). + Expect().Status(http.StatusBadRequest). + JSON().Object().ContainsKey("error") + }) +} diff --git a/deployment/terraform/agent.go b/deployment/terraform/agent.go index 1484ad3ea..9f55cdaf0 100644 --- a/deployment/terraform/agent.go +++ b/deployment/terraform/agent.go @@ -15,6 +15,7 @@ import ( "github.com/mattermost/mattermost-load-test-ng/deployment" "github.com/mattermost/mattermost-load-test-ng/deployment/terraform/ssh" "github.com/mattermost/mattermost-load-test-ng/loadtest" + "github.com/mattermost/mattermost-load-test-ng/loadtest/control/browsercontroller" "github.com/mattermost/mattermost/server/public/shared/mlog" ) @@ -104,6 +105,22 @@ func (t *Terraform) configureAndRunAgents(extAgent *ssh.ExtAgent) error { index int } + // We read the local browsercontroller.json file, marshal it, and upload it to each browser agent below so + // that ltbrowserapi has a valid config at startup. Thus, local browsercontroller.json config file is required + // for browser load tests to work. + var browserControllerConfig string + if t.output.HasBrowserAgents() { + bccfg, err := browsercontroller.ReadConfig("./config/browsercontroller.json") + if err != nil { + return fmt.Errorf("error reading browser controller config: %w", err) + } + data, err := json.MarshalIndent(bccfg, "", " ") + if err != nil { + return fmt.Errorf("error marshaling browser controller config: %w", err) + } + browserControllerConfig = string(data) + } + allAgents := make([]agentInfo, 0, len(t.output.Agents)+len(t.output.BrowserAgents)) for i, agent := range t.output.Agents { allAgents = append(allAgents, agentInfo{instance: agent, agentType: deployment.AgentTypeServer, index: i}) @@ -210,6 +227,15 @@ func (t *Terraform) configureAndRunAgents(extAgent *ssh.ExtAgent) error { batch = append(batch, uploadInfo{srcData: strings.Join(splitFiles[agentNumber], "\n"), dstPath: t.ExpandWithUser(dstUsersFilePath), msg: "Uploading list of users credentials"}) } + // Upload the browsercontroller.json to the browser agent instance. + if agentType == deployment.AgentTypeBrowser { + batch = append(batch, uploadInfo{ + srcData: browserControllerConfig, + dstPath: t.ExpandWithUser("/home/{{.Username}}/mattermost-load-test-ng/config/browsercontroller.json"), + msg: "Uploading browsercontroller.json", + }) + } + // If SiteURL is set, update /etc/hosts to point to the correct IP if t.config.SiteURL != "" { appHostsFile, err := t.getAppHostsFile(agentNumber) From ca6e036f26765aee5af43e766455830e13259c65 Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 20 Mar 2026 05:26:29 +0530 Subject: [PATCH 06/11] fix test --- api/agent_client_test.go | 15 +++++++++++++++ api/client_test.go | 5 +++++ api/coordinator_client_test.go | 8 ++++++++ api/coordinator_test.go | 2 ++ 4 files changed, 30 insertions(+) diff --git a/api/agent_client_test.go b/api/agent_client_test.go index ce01e0e6f..2f42ca2ba 100644 --- a/api/agent_client_test.go +++ b/api/agent_client_test.go @@ -13,6 +13,7 @@ import ( client "github.com/mattermost/mattermost-load-test-ng/api/client/agent" "github.com/mattermost/mattermost-load-test-ng/defaults" + "github.com/mattermost/mattermost-load-test-ng/deployment" "github.com/mattermost/mattermost-load-test-ng/loadtest" "github.com/mattermost/mattermost-load-test-ng/loadtest/control" "github.com/mattermost/mattermost-load-test-ng/loadtest/control/simulcontroller" @@ -64,6 +65,8 @@ func createAgent(t *testing.T, id, serverURL string) *client.Agent { } func TestCreateAgent(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -155,6 +158,8 @@ func TestCreateAgent(t *testing.T) { } func TestAgentId(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -168,6 +173,8 @@ func TestAgentId(t *testing.T) { } func TestAgentStatus(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -185,6 +192,8 @@ func TestAgentStatus(t *testing.T) { } func TestAgentRunStop(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -245,6 +254,8 @@ func TestAgentRunStop(t *testing.T) { } func TestAgentAddRemoveUsers(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -301,6 +312,8 @@ func TestAgentAddRemoveUsers(t *testing.T) { } func TestAgentDestroy(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -332,6 +345,8 @@ func TestAgentDestroy(t *testing.T) { } func TestAgentInjectAction(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) diff --git a/api/client_test.go b/api/client_test.go index 966ff0793..0640dbd6a 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -14,6 +14,7 @@ import ( coordClient "github.com/mattermost/mattermost-load-test-ng/api/client/coordinator" "github.com/mattermost/mattermost-load-test-ng/coordinator" "github.com/mattermost/mattermost-load-test-ng/defaults" + "github.com/mattermost/mattermost-load-test-ng/deployment" "github.com/mattermost/mattermost-load-test-ng/loadtest" "github.com/mattermost/mattermost-load-test-ng/loadtest/control" "github.com/mattermost/mattermost-load-test-ng/loadtest/control/simulcontroller" @@ -26,6 +27,8 @@ import ( const n = 8 func TestAgentClientConcurrency(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -178,6 +181,8 @@ func TestAgentClientConcurrency(t *testing.T) { } func TestCoordClientConcurrency(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) diff --git a/api/coordinator_client_test.go b/api/coordinator_client_test.go index fea972a82..f9a9e7d13 100644 --- a/api/coordinator_client_test.go +++ b/api/coordinator_client_test.go @@ -11,6 +11,7 @@ import ( client "github.com/mattermost/mattermost-load-test-ng/api/client/coordinator" "github.com/mattermost/mattermost-load-test-ng/coordinator" "github.com/mattermost/mattermost-load-test-ng/defaults" + "github.com/mattermost/mattermost-load-test-ng/deployment" "github.com/mattermost/mattermost-load-test-ng/loadtest" "github.com/mattermost/mattermost-load-test-ng/loadtest/control" "github.com/mattermost/mattermost-load-test-ng/logger" @@ -38,6 +39,8 @@ func createCoordinator(t *testing.T, id, serverURL string) *client.Coordinator { } func TestCreateCoordinator(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) + // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -95,6 +98,7 @@ func TestCreateCoordinator(t *testing.T) { } func TestCoordinatorId(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -108,6 +112,7 @@ func TestCoordinatorId(t *testing.T) { } func TestCoordinatorStatus(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -125,6 +130,7 @@ func TestCoordinatorStatus(t *testing.T) { } func TestCoordinatorStartStop(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -196,6 +202,7 @@ func TestCoordinatorStartStop(t *testing.T) { } func TestCoordinatorDestroy(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) @@ -226,6 +233,7 @@ func TestCoordinatorDestroy(t *testing.T) { } func TestCoordinatorInjectAction(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) diff --git a/api/coordinator_test.go b/api/coordinator_test.go index 56340f83c..6329fb220 100644 --- a/api/coordinator_test.go +++ b/api/coordinator_test.go @@ -10,6 +10,7 @@ import ( "github.com/mattermost/mattermost-load-test-ng/coordinator" "github.com/mattermost/mattermost-load-test-ng/defaults" + "github.com/mattermost/mattermost-load-test-ng/deployment" "github.com/mattermost/mattermost-load-test-ng/loadtest" "github.com/mattermost/mattermost-load-test-ng/loadtest/control" "github.com/mattermost/mattermost-load-test-ng/logger" @@ -19,6 +20,7 @@ import ( ) func TestCoordinatorAPI(t *testing.T) { + setupAgentType(t, deployment.AgentTypeServer) // create http.Handler handler := SetupAPIRouter(logger.New(&logger.Settings{}), logger.New(&logger.Settings{})) From b782f61c1c82db4d8abad4ad591579a7a5da7e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?= Date: Fri, 20 Mar 2026 13:24:49 +0100 Subject: [PATCH 07/11] Fix test with valid config file --- api/agent_test.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/api/agent_test.go b/api/agent_test.go index ac6b08365..92b4b09e2 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -400,11 +400,32 @@ func TestBrowserAgentConfigValidation(t *testing.T) { t.Run("succeeds with valid browsercontroller.json", func(t *testing.T) { setupAgentType(t, deployment.AgentTypeBrowser) - // ReadConfig uses "./config/browsercontroller.json" relative to cwd, - // so we need to be at the repo root for the path to resolve. + // Create a temporary directory with a valid browsercontroller.json + tempDir, err := os.MkdirTemp("", "test_browser_valid_config") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tempDir) }) + + configDir := filepath.Join(tempDir, "config") + require.NoError(t, os.MkdirAll(configDir, 0755)) + + validConfig := `{ + "SimulationId": "mattermostPostAndScroll", + "RunInHeadless": true, + "SimulationTimeoutMs": 60000, + "EnabledPlugins": false, + "LogSettings": { + "EnableConsole": true, + "ConsoleLevel": "debug", + "EnableFile": true, + "FileLevel": "debug", + "FileLocation": "browseragent.log" + } + }` + require.NoError(t, os.WriteFile(filepath.Join(configDir, "browsercontroller.json"), []byte(validConfig), 0644)) + originalDir, err := os.Getwd() require.NoError(t, err) - require.NoError(t, os.Chdir("..")) + require.NoError(t, os.Chdir(tempDir)) t.Cleanup(func() { os.Chdir(originalDir) }) rd := requestData{LoadTestConfig: ltConfig} From 86acb00aab3dfcc201be438ed37d1c2ece0d9200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?= Date: Fri, 20 Mar 2026 13:25:31 +0100 Subject: [PATCH 08/11] Fix typo in comment --- browser/src/config/accessors.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/src/config/accessors.ts b/browser/src/config/accessors.ts index 80ac9e70e..63feb0624 100644 --- a/browser/src/config/accessors.ts +++ b/browser/src/config/accessors.ts @@ -5,7 +5,7 @@ import {browserControllerConfigJson} from './loader.js'; /** * Server URl is always passed as a parameter to the browser controller while - * its created. So we don't need to read it from the config.json. But we need + * it's created. So we don't need to read it from the config.json. But we need * hardcoded value for tests and smoke simulations. */ export function getMattermostServerURL(): string { From 11aec15ac22ee31fa34ee0f815476d98e884feab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?= Date: Fri, 20 Mar 2026 13:26:57 +0100 Subject: [PATCH 09/11] Fix typo in docs --- docs/config/browsercontroller.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/config/browsercontroller.md b/docs/config/browsercontroller.md index eecc97b60..112ef5cc6 100644 --- a/docs/config/browsercontroller.md +++ b/docs/config/browsercontroller.md @@ -52,7 +52,7 @@ When true, the browser server outputs log messages to the console based on Conso Level of detail at which log events are written to the console. -Possible values (in order of decreasing verbosity, these are case sensitive): +Possible values (in order of decreasing verbosity, these are case-sensitive): - `trace` - `debug` - `info` @@ -72,7 +72,7 @@ When true, the browser server outputs log messages to the file specified by the Level of detail at which log events are written to log files. Exactly same as `ConsoleLevel` as mentioned above. -Possible values (in order of decreasing verbosity, these are case sensitive): +Possible values (in order of decreasing verbosity, these are case-sensitive): - `trace` - `debug` - `info` From 251b21ce599b271cee6f5d9638955481534c45fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?= Date: Fri, 20 Mar 2026 13:40:47 +0100 Subject: [PATCH 10/11] Use t.Chdir and t.Setenv instead of os's This takes care of cleanup automatically and marks the test as not suitable for test parallelism. --- api/agent_test.go | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/api/agent_test.go b/api/agent_test.go index 92b4b09e2..d4efbbc83 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -34,16 +34,13 @@ type requestData struct { func setupAgentType(t *testing.T, agentType string) { t.Helper() - originalHome := os.Getenv("HOME") tempDir, err := os.MkdirTemp("", "test_home_"+agentType) require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tempDir) }) + agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) require.NoError(t, os.WriteFile(agentTypeFile, []byte(agentType), 0644)) - os.Setenv("HOME", tempDir) - t.Cleanup(func() { - os.Setenv("HOME", originalHome) - os.RemoveAll(tempDir) - }) + t.Setenv("HOME", tempDir) } func TestAgentAPI(t *testing.T) { @@ -338,14 +335,10 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt file does not exist", func(t *testing.T) { - originalHome := os.Getenv("HOME") tempDir, err := os.MkdirTemp("", "test_home_missing") require.NoError(t, err) - t.Cleanup(func() { - os.Setenv("HOME", originalHome) - os.RemoveAll(tempDir) - }) - os.Setenv("HOME", tempDir) + t.Cleanup(func() { os.RemoveAll(tempDir) }) + t.Setenv("HOME", tempDir) result, err := isBrowserAgentInstance() require.Error(t, err) @@ -353,14 +346,10 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt contains unknown content", func(t *testing.T) { - originalHome := os.Getenv("HOME") tempDir, err := os.MkdirTemp("", "test_home_unknown") require.NoError(t, err) - t.Cleanup(func() { - os.Setenv("HOME", originalHome) - os.RemoveAll(tempDir) - }) - os.Setenv("HOME", tempDir) + t.Cleanup(func() { os.RemoveAll(tempDir) }) + t.Setenv("HOME", tempDir) agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) require.NoError(t, os.WriteFile(agentTypeFile, []byte("unknown_agent_type"), 0644)) @@ -423,10 +412,7 @@ func TestBrowserAgentConfigValidation(t *testing.T) { }` require.NoError(t, os.WriteFile(filepath.Join(configDir, "browsercontroller.json"), []byte(validConfig), 0644)) - originalDir, err := os.Getwd() - require.NoError(t, err) - require.NoError(t, os.Chdir(tempDir)) - t.Cleanup(func() { os.Chdir(originalDir) }) + t.Chdir(tempDir) rd := requestData{LoadTestConfig: ltConfig} obj := e.POST("/create").WithQuery("id", "ltb1").WithJSON(rd). @@ -465,10 +451,7 @@ func TestBrowserAgentConfigValidation(t *testing.T) { }` require.NoError(t, os.WriteFile(filepath.Join(configDir, "browsercontroller.json"), []byte(invalidConfig), 0644)) - originalDir, err := os.Getwd() - require.NoError(t, err) - require.NoError(t, os.Chdir(tempDir)) - t.Cleanup(func() { os.Chdir(originalDir) }) + t.Chdir(tempDir) rd := requestData{LoadTestConfig: ltConfig} e.POST("/create").WithQuery("id", "ltb2").WithJSON(rd). From 05d413de56dcf0e953977b14fc8e5861466af1b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?= Date: Fri, 20 Mar 2026 13:56:37 +0100 Subject: [PATCH 11/11] Use t.TempDir instead of os.MkdirTemp The testing version takes care of cleanup automatically. --- api/agent_test.go | 52 ++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/api/agent_test.go b/api/agent_test.go index d4efbbc83..f57e27d61 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -34,10 +34,7 @@ type requestData struct { func setupAgentType(t *testing.T, agentType string) { t.Helper() - tempDir, err := os.MkdirTemp("", "test_home_"+agentType) - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tempDir) }) - + tempDir := t.TempDir() agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) require.NoError(t, os.WriteFile(agentTypeFile, []byte(agentType), 0644)) t.Setenv("HOME", tempDir) @@ -335,9 +332,7 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt file does not exist", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "test_home_missing") - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tempDir) }) + tempDir := t.TempDir() t.Setenv("HOME", tempDir) result, err := isBrowserAgentInstance() @@ -346,9 +341,7 @@ func TestIsBrowserAgentInstance(t *testing.T) { }) t.Run("returns false when agent_type.txt contains unknown content", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "test_home_unknown") - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tempDir) }) + tempDir := t.TempDir() t.Setenv("HOME", tempDir) agentTypeFile := filepath.Join(tempDir, deployment.AgentTypeFileName) @@ -377,9 +370,22 @@ func TestBrowserAgentConfigValidation(t *testing.T) { ltConfig.ConnectionConfiguration.ServerURL = mmServer.URL ltConfig.UsersConfiguration.MaxActiveUsers = 100 + setupBaseCfgDir := func(t *testing.T) string { + t.Helper() + tempDir := t.TempDir() + t.Chdir(tempDir) + cfgDir := filepath.Join(tempDir, "config") + require.NoError(t, os.MkdirAll(cfgDir, 0755)) + + return cfgDir + } + t.Run("fails when browsercontroller.json is missing", func(t *testing.T) { setupAgentType(t, deployment.AgentTypeBrowser) + // Empty config directory + _ = setupBaseCfgDir(t) + rd := requestData{LoadTestConfig: ltConfig} e.POST("/create").WithQuery("id", "ltb0").WithJSON(rd). Expect().Status(http.StatusBadRequest). @@ -389,13 +395,8 @@ func TestBrowserAgentConfigValidation(t *testing.T) { t.Run("succeeds with valid browsercontroller.json", func(t *testing.T) { setupAgentType(t, deployment.AgentTypeBrowser) - // Create a temporary directory with a valid browsercontroller.json - tempDir, err := os.MkdirTemp("", "test_browser_valid_config") - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tempDir) }) - - configDir := filepath.Join(tempDir, "config") - require.NoError(t, os.MkdirAll(configDir, 0755)) + // Config directory with a valid browsercontroller.json + cfgDir := setupBaseCfgDir(t) validConfig := `{ "SimulationId": "mattermostPostAndScroll", @@ -410,9 +411,7 @@ func TestBrowserAgentConfigValidation(t *testing.T) { "FileLocation": "browseragent.log" } }` - require.NoError(t, os.WriteFile(filepath.Join(configDir, "browsercontroller.json"), []byte(validConfig), 0644)) - - t.Chdir(tempDir) + require.NoError(t, os.WriteFile(filepath.Join(cfgDir, "browsercontroller.json"), []byte(validConfig), 0644)) rd := requestData{LoadTestConfig: ltConfig} obj := e.POST("/create").WithQuery("id", "ltb1").WithJSON(rd). @@ -428,13 +427,8 @@ func TestBrowserAgentConfigValidation(t *testing.T) { t.Run("fails with invalid browsercontroller.json values", func(t *testing.T) { setupAgentType(t, deployment.AgentTypeBrowser) - // Create a temporary directory with an invalid browsercontroller.json - tempDir, err := os.MkdirTemp("", "test_browser_invalid_config") - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tempDir) }) - - configDir := filepath.Join(tempDir, "config") - require.NoError(t, os.MkdirAll(configDir, 0755)) + // Config directory with an invalid browsercontroller.json + cfgDir := setupBaseCfgDir(t) invalidConfig := `{ "SimulationId": "", @@ -449,9 +443,7 @@ func TestBrowserAgentConfigValidation(t *testing.T) { "FileLocation": "browseragent.log" } }` - require.NoError(t, os.WriteFile(filepath.Join(configDir, "browsercontroller.json"), []byte(invalidConfig), 0644)) - - t.Chdir(tempDir) + require.NoError(t, os.WriteFile(filepath.Join(cfgDir, "browsercontroller.json"), []byte(invalidConfig), 0644)) rd := requestData{LoadTestConfig: ltConfig} e.POST("/create").WithQuery("id", "ltb2").WithJSON(rd).