diff --git a/lib/npm.js b/lib/npm.js index 9b00321556a44..892cf4b96c8a0 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -28,6 +28,7 @@ class Npm { return require(`./commands/${command}.js`) } + unrefPromises = [] updateNotification = null argv = [] @@ -224,11 +225,16 @@ class Npm { log.verbose('argv', this.#argvClean.map(JSON.stringify).join(' ')) }) - this.#logFile.load({ + // logFile.load returns a promise that resolves when old logs are done being cleaned. + // We save this promise to an array so that we can await it in tests to ensure more + // deterministic logging behavior. The process will also hang open if this were to + // take a long time to resolve, but that is why process.exit is called explicitly + // in the exit-handler. + this.unrefPromises.push(this.#logFile.load({ path: this.logPath, logsMax: this.config.get('logs-max'), timing: this.config.get('timing'), - }) + })) this.#timers.load({ path: this.logPath, diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 4a58b869858fd..5618a12566003 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -73,6 +73,17 @@ const getMockNpm = async (t, { mocks, init, load, npm: npmOpts }) => { }) } + async load () { + const res = await super.load() + // Wait for any promises (currently only log file cleaning) to be + // done before returning from load in tests. This helps create more + // deterministic testing behavior because in reality that promise + // is left hanging on purpose as a best-effort and the process gets + // closed regardless of if it has finished or not. + await Promise.all(this.unrefPromises) + return res + } + async exec (...args) { const [res, err] = await super.exec(...args).then((r) => [r]).catch(e => [null, e]) // This mimics how the exit handler flushes output for commands that have diff --git a/test/lib/cli/exit-handler.js b/test/lib/cli/exit-handler.js index 3cb4523b3ee51..fd6754b30096e 100644 --- a/test/lib/cli/exit-handler.js +++ b/test/lib/cli/exit-handler.js @@ -129,8 +129,6 @@ t.test('handles unknown error with logs and debug file', async (t) => { }) await exitHandler(err('Unknown error', 'ECODE')) - // force logfile cleaning logs to happen since those are purposefully not awaited - await require('timers/promises').setTimeout(200) const fileLogs = await debugFile() const fileLines = fileLogs.split('\n') @@ -140,19 +138,14 @@ t.test('handles unknown error with logs and debug file', async (t) => { t.equal(process.exitCode, 1) - let skippedLogs = 0 logs.forEach((logItem, i) => { const logLines = logItem.split('\n').map(l => `${i} ${l}`) for (const line of logLines) { - if (line.includes('logfile') && line.includes('cleaning')) { - skippedLogs++ - continue - } t.match(fileLogs.trim(), line, 'log appears in debug file') } }) - t.equal(logs.length - skippedLogs, parseInt(lastLog) + 1) + t.equal(logs.length, parseInt(lastLog) + 1) t.match(logs.error, [ 'code ECODE', 'ERR SUMMARY Unknown error',