diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index b9af6f9bd..054bfed34 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -324,6 +324,7 @@ describe('CliRepl', function () { 'disableLogging', 'logLocation', 'logRetentionDays', + 'logMaxFileCount', ] satisfies (keyof CliUserConfig)[]); }); @@ -1457,6 +1458,25 @@ describe('CliRepl', function () { testRetentionDays ); }); + + it('can set log max file count', async function () { + const testMaxFileCount = 123; + cliRepl.config.logMaxFileCount = testMaxFileCount; + const oldEnvironmentLimit = + process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT; + delete process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT; + await cliRepl.start(await testServer.connectionString(), {}); + + expect(await cliRepl.getConfig('logMaxFileCount')).equals( + testMaxFileCount + ); + expect(cliRepl.logManager?._options.maxLogFileCount).equals( + testMaxFileCount + ); + + process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT = + oldEnvironmentLimit; + }); }); it('times out fast', async function () { diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 88e05a428..2cb3214d9 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -261,7 +261,8 @@ export class CliRepl implements MongoshIOProvider { this.shellHomeDirectory.localPath('.'), retentionDays: await this.getConfig('logRetentionDays'), maxLogFileCount: +( - process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT || 100 + process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT || + (await this.getConfig('logMaxFileCount')) ), onerror: (err: Error) => this.bus.emit('mongosh:error', err, 'log'), onwarn: (err: Error, path: string) => diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index 93c8cc373..5bb38a1d5 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -1747,6 +1747,58 @@ describe('e2e', function () { }); }); + describe('with custom log retention max file count', function () { + const customLogDir = useTmpdir(); + + it('should delete files once it is above the max file count limit', async function () { + const globalConfig = path.join(homedir, 'globalconfig.conf'); + await fs.writeFile( + globalConfig, + `mongosh:\n logLocation: ${JSON.stringify( + customLogDir.path + )}\n logMaxFileCount: 4` + ); + const paths: string[] = []; + const offset = Math.floor(Date.now() / 1000); + + // Create 10 log files + for (let i = 9; i >= 0; i--) { + const filename = path.join( + customLogDir.path, + ObjectId.createFromTime(offset - i).toHexString() + '_log' + ); + await fs.writeFile(filename, ''); + paths.push(filename); + } + + // All 10 existing log files exist. + expect(await getFilesState(paths)).to.equal('1111111111'); + shell = this.startTestShell({ + args: ['--nodb'], + env: { + ...env, + MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT: '', + MONGOSH_GLOBAL_CONFIG_FILE_FOR_TESTING: globalConfig, + }, + forceTerminal: true, + }); + + await shell.waitForPrompt(); + + // Add the newly created log to the file list. + paths.push( + path.join(customLogDir.path, `${shell.logId as string}_log`) + ); + + expect( + await shell.executeLine('config.get("logMaxFileCount")') + ).contains('4'); + + // Expect 7 files to be deleted and 4 to remain (including the new log file) + expect(await getFilesState(paths)).to.equal('00000001111'); + }); + }); + it('creates a log file that keeps track of session events', async function () { expect(await shell.executeLine('print(123 + 456)')).to.include('579'); const log = await readLogFile(); @@ -1816,23 +1868,26 @@ describe('e2e', function () { const newLogId = shell.logId; expect(newLogId).not.null; expect(oldLogId).equals(newLogId); - log = await readLogFile(); - expect( - log.filter( - (logEntry) => logEntry.attr?.input === 'print(111 + 222)' - ) - ).to.have.lengthOf(1); - expect( - log.filter( - (logEntry) => logEntry.attr?.input === 'print(579 - 123)' - ) - ).to.have.lengthOf(1); - expect( - log.filter( - (logEntry) => logEntry.attr?.input === 'print(123 + 456)' - ) - ).to.have.lengthOf(0); + await eventually(async () => { + log = await readLogFile(); + + expect( + log.filter( + (logEntry) => logEntry.attr?.input === 'print(111 + 222)' + ) + ).to.have.lengthOf(1); + expect( + log.filter( + (logEntry) => logEntry.attr?.input === 'print(579 - 123)' + ) + ).to.have.lengthOf(1); + expect( + log.filter( + (logEntry) => logEntry.attr?.input === 'print(123 + 456)' + ) + ).to.have.lengthOf(0); + }); }); it('includes information about the driver version', async function () { diff --git a/packages/types/src/index.spec.ts b/packages/types/src/index.spec.ts index 139312935..7d1322f1b 100644 --- a/packages/types/src/index.spec.ts +++ b/packages/types/src/index.spec.ts @@ -31,6 +31,12 @@ describe('config validation', function () { expect(await validate('logRetentionDays', -1)).to.equal( 'logRetentionDays must be a positive integer' ); + expect(await validate('logMaxFileCount', 'foo')).to.equal( + 'logMaxFileCount must be a positive integer' + ); + expect(await validate('logMaxFileCount', -1)).to.equal( + 'logMaxFileCount must be a positive integer' + ); expect(await validate('showStackTraces', 'foo')).to.equal( 'showStackTraces must be a boolean' ); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index bdf495f93..ab8134bdb 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -510,6 +510,7 @@ export class CliUserConfig extends SnippetShellUserConfig { disableLogging = false; logLocation: string | undefined = undefined; logRetentionDays = 30; + logMaxFileCount = 100; } export class CliUserConfigValidator extends SnippetShellUserConfigValidator { @@ -533,6 +534,7 @@ export class CliUserConfigValidator extends SnippetShellUserConfigValidator { case 'inspectDepth': case 'historyLength': case 'logRetentionDays': + case 'logMaxFileCount': if (typeof value !== 'number' || value < 0) { return `${key} must be a positive integer`; }