feat: add logs-dir config to set custom logging location

This also allows logs-max to be set to 0 to disable log file writing.

Closes #4466
Closes #4206

1 files changed, 56 insertions, 22 deletions

diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js
index 6a96d92dd..73bbf06fe 100644
--- a/test/lib/utils/exit-handler.js
+++ b/test/lib/utils/exit-handler.js
@@ -21,9 +21,10 @@ t.formatSnapshot = (obj) => {
 }
 
 t.cleanSnapshot = (path) => cleanDate(cleanCwd(path))
-// Config loading is dependent on env so strip those from snapshots
+  // Config loading is dependent on env so strip those from snapshots
   .replace(/.*timing config:load:.*\n/gm, '')
   .replace(/(Completed in )\d+(ms)/g, '$1{TIME}$2')
+  .replace(/(removing )\d+( files)/g, '$1${NUM}2')
 
 // cut off process from script so that it won't quit the test runner
 // while trying to run through the myriad of cases.  need to make it
@@ -44,9 +45,8 @@ mockGlobals(t, {
   }),
 }, { replace: true })
 
-const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
+const mockExitHandler = async (t, { init, load, testdir, config, globals, mocks } = {}) => {
   const errors = []
-  mockGlobals(t, { 'console.error': (err) => errors.push(err) })
 
   const { npm, logMocks, ...rest } = await loadMockNpm(t, {
     init,
@@ -56,11 +56,15 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
       '../../package.json': {
         version: '1.0.0',
       },
+      ...mocks,
     },
     config: {
       loglevel: 'notice',
       ...config,
     },
+    globals: {
+      'console.error': (err) => errors.push(err),
+    },
   })
 
   const exitHandler = t.mock('../../../lib/utils/exit-handler.js', {
@@ -74,6 +78,7 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
       release: () => '1.0.0',
     },
     ...logMocks,
+    ...mocks,
   })
 
   if (npm) {
@@ -89,13 +94,14 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
     ...rest,
     errors,
     npm,
-    // Make it async to make testing ergonomics a little
-    // easier so we dont need to t.plan() every test to
-    // make sure we get process.exit called
-    exitHandler: (...args) => new Promise(resolve => {
+    // Make it async to make testing ergonomics a little easier so we dont need
+    // to t.plan() every test to make sure we get process.exit called. Also
+    // introduce a small artificial delay so the logs are consistently finished
+    // by the time the exit handler forces process.exit
+    exitHandler: (...args) => new Promise(resolve => setTimeout(() => {
       process.once('exit', resolve)
       exitHandler(...args)
-    }),
+    }, 50)),
   }
 }
 
@@ -199,17 +205,15 @@ t.test('exit handler called - no npm with error without stack', async (t) => {
 })
 
 t.test('console.log output using --json', async (t) => {
-  const { exitHandler, errors } = await mockExitHandler(t, {
-    config: {
-      json: true,
-    },
+  const { exitHandler, outputErrors } = await mockExitHandler(t, {
+    config: { json: true },
   })
 
   await exitHandler(err('Error: EBADTHING Something happened'))
 
   t.equal(process.exitCode, 1)
   t.same(
-    JSON.parse(errors[0]),
+    JSON.parse(outputErrors[0]),
     {
       error: {
         code: 'EBADTHING', // should default error code to E[A-Z]+
@@ -273,11 +277,43 @@ t.test('npm.config not ready', async (t) => {
   ], 'should exit with config error msg')
 })
 
-t.test('timing with no error', async (t) => {
-  const { exitHandler, timingFile, npm, logs } = await mockExitHandler(t, {
+t.test('no logs dir', async (t) => {
+  const { exitHandler, logs } = await mockExitHandler(t, {
+    config: { 'logs-max': 0 },
+  })
+
+  await exitHandler(new Error())
+
+  t.match(logs.error.filter(([t]) => t === ''), [
+    ['', 'Log files were not written due to the config logs-max=0'],
+  ])
+})
+
+t.test('log file error', async (t) => {
+  const { exitHandler, logs } = await mockExitHandler(t, {
     config: {
+      'logs-dir': 'LOGS_DIR',
       timing: true,
     },
+    mocks: {
+      '@npmcli/fs': {
+        mkdir: async (dir) => {
+          if (dir.includes('LOGS_DIR')) {
+            throw new Error('err')
+          }
+        },
+      },
+    },
+  })
+
+  await exitHandler(new Error())
+
+  t.match(logs.error.filter(([t]) => t === ''), [['', `error writing to the directory`]])
+})
+
+t.test('timing with no error', async (t) => {
+  const { exitHandler, timingFile, npm, logs } = await mockExitHandler(t, {
+    config: { timing: true },
   })
 
   await exitHandler()
@@ -285,9 +321,9 @@ t.test('timing with no error', async (t) => {
 
   t.equal(process.exitCode, 0)
 
-  t.match(logs.error, [
-    ['', /A complete log of this run can be found in:[\s\S]*-debug-\d\.log/],
-  ])
+  const msg = logs.info.filter(([t]) => t === '')[0][1]
+  t.match(msg, /A complete log of this run can be found in:/)
+  t.match(msg, /Timing info written to:/)
 
   t.match(
     timingFileData,
@@ -308,9 +344,7 @@ t.test('timing with no error', async (t) => {
 
 t.test('unfinished timers', async (t) => {
   const { exitHandler, timingFile, npm } = await mockExitHandler(t, {
-    config: {
-      timing: true,
-    },
+    config: { timing: true },
   })
 
   process.emit('time', 'foo')
@@ -376,7 +410,7 @@ t.test('verbose logs replace info on err props', async t => {
   await exitHandler(err('Error with code type number', properties))
   t.equal(process.exitCode, 1)
   t.match(
-    logs.verbose.filter(([p]) => p !== 'logfile'),
+    logs.verbose.filter(([p]) => !['logfile', 'title', 'argv'].includes(p)),
     keys.map((k) => [k, `${k}-https://user:***@registry.npmjs.org/`]),
     'all special keys get replaced'
   )