From 9905d0e24c162c3f6cc006fa86b4c9d0205a4c6f Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 21 Jul 2022 09:40:55 -0700 Subject: fix: don't fail immediately if cache dir is not accessible (#5197) This also changes all the log messages about not being able to create initial directories and files to `log.verbose` since we know run those commands on init. There are a lot of valid reasons why those might fail, and we don't want to show a warning for them every time. Fixes: #4769 Fixes: #4838 Fixes: #4996 --- test/fixtures/mock-npm.js | 36 ++++++++++++++++++------------------ test/lib/npm.js | 46 +++++++++++++++++++++++++++++++++------------- 2 files changed, 51 insertions(+), 31 deletions(-) (limited to 'test') diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index a79812fb7..90bf7da4c 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -108,17 +108,20 @@ const LoadMockNpm = async (t, { cache: cacheDir, global: globalPrefixDir, }) - const prefix = path.join(dir, 'prefix') - const cache = path.join(dir, 'cache') - const globalPrefix = path.join(dir, 'global') - const home = path.join(dir, 'home') + const dirs = { + testdir: dir, + prefix: path.join(dir, 'prefix'), + cache: path.join(dir, 'cache'), + globalPrefix: path.join(dir, 'global'), + home: path.join(dir, 'home'), + } // Set cache to testdir via env var so it is available when load is run // XXX: remove this for a solution where cache argv is passed in mockGlobals(t, { - 'process.env.HOME': home, - 'process.env.npm_config_cache': cache, - ...(globals ? result(globals, { prefix, cache, home }) : {}), + 'process.env.HOME': dirs.home, + 'process.env.npm_config_cache': dirs.cache, + ...(globals ? result(globals, { ...dirs }) : {}), // Some configs don't work because they can't be set via npm.config.set until // config is loaded. But some config items are needed before that. So this is // an explicit set of configs that must be loaded as env vars. @@ -126,7 +129,8 @@ const LoadMockNpm = async (t, { ...Object.entries(config) .filter(([k]) => envConfigKeys.includes(k)) .reduce((acc, [k, v]) => { - acc[`process.env.npm_config_${k.replace(/-/g, '_')}`] = v.toString() + acc[`process.env.npm_config_${k.replace(/-/g, '_')}`] = + result(v, { ...dirs }).toString() return acc }, {}), }) @@ -138,7 +142,7 @@ const LoadMockNpm = async (t, { if (load) { await npm.load() - for (const [k, v] of Object.entries(result(config, { npm, prefix, cache }))) { + for (const [k, v] of Object.entries(result(config, { npm, ...dirs }))) { if (typeof v === 'object' && v.value && v.where) { npm.config.set(k, v.value, v.where) } else { @@ -148,20 +152,16 @@ const LoadMockNpm = async (t, { // Set global loglevel *again* since it possibly got reset during load // XXX: remove with npmlog setLoglevel(t, config.loglevel, false) - npm.prefix = prefix - npm.cache = cache - npm.globalPrefix = globalPrefix + npm.prefix = dirs.prefix + npm.cache = dirs.cache + npm.globalPrefix = dirs.globalPrefix } return { ...rest, + ...dirs, Npm, npm, - home, - prefix, - globalPrefix, - testdir: dir, - cache, debugFile: async () => { const readFiles = npm.logFiles.map(f => fs.readFile(f)) const logFiles = await Promise.all(readFiles) @@ -171,7 +171,7 @@ const LoadMockNpm = async (t, { .join('\n') }, timingFile: async () => { - const data = await fs.readFile(path.resolve(cache, '_timing.json'), 'utf8') + const data = await fs.readFile(path.resolve(dirs.cache, '_timing.json'), 'utf8') return JSON.parse(data) // XXX: this fails if multiple timings are written }, } diff --git a/test/lib/npm.js b/test/lib/npm.js index cd692a93f..62e48ce60 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -3,6 +3,7 @@ const { resolve, dirname, join } = require('path') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') const mockGlobals = require('../fixtures/mock-globals') +const fs = require('@npmcli/fs') // delete this so that we don't have configs from the fact that it // is being run by 'npm test' @@ -435,23 +436,42 @@ t.test('debug log', async t => { t.match(debug, log2.join(' '), 'after load log appears') }) - t.test('with bad dir', async t => { - const { npm } = await loadMockNpm(t, { + t.test('can load with bad dir', async t => { + const { npm, testdir } = await loadMockNpm(t, { + load: false, config: { - 'logs-dir': 'LOGS_DIR', - }, - mocks: { - '@npmcli/fs': { - mkdir: async (dir) => { - if (dir.includes('LOGS_DIR')) { - throw new Error('err') - } - }, - }, + 'logs-dir': (c) => join(c.testdir, 'my_logs_dir'), }, }) + const logsDir = join(testdir, 'my_logs_dir') + + // make logs dir a file before load so it files + await fs.writeFile(logsDir, 'A_TEXT_FILE') + await t.resolves(npm.load(), 'loads with invalid logs dir') + + t.equal(npm.logFiles.length, 0, 'no log files array') + t.strictSame(fs.readFileSync(logsDir, 'utf-8'), 'A_TEXT_FILE') + }) +}) + +t.test('cache dir', async t => { + t.test('creates a cache dir', async t => { + const { npm } = await loadMockNpm(t) + + t.ok(fs.existsSync(npm.cache), 'cache dir exists') + }) + + t.test('can load with a bad cache dir', async t => { + const { npm, cache } = await loadMockNpm(t, { + load: false, + // The easiest way to make mkdir(cache) fail is to make it a file. + // This will have the same effect as if its read only or inaccessible. + cacheDir: 'A_TEXT_FILE', + }) + + await t.resolves(npm.load(), 'loads with cache dir as a file') - t.equal(npm.logFiles.length, 0, 'no log file') + t.equal(fs.readFileSync(cache, 'utf-8'), 'A_TEXT_FILE') }) }) -- cgit v1.2.3