From 1e977eec223463b04a6ad2c1953e31a6d5d22f2c Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Tue, 23 May 2023 08:39:41 -0700 Subject: [PATCH] chore: add smoke-test that will use current npm source to overwrite itself (#6491) --- .github/workflows/ci-release.yml | 18 +- DEPENDENCIES.md | 2 - package-lock.json | 2 - scripts/remove-files.js | 11 - scripts/template-oss/ci-release.yml | 18 +- smoke-tests/package.json | 2 - .../tap-snapshots/test/index.js.test.cjs | 2 +- smoke-tests/test/fixtures/setup.js | 221 ++++++++++-------- smoke-tests/test/index.js | 26 ++- smoke-tests/test/npm-replace-global.js | 82 +++++++ 10 files changed, 262 insertions(+), 122 deletions(-) delete mode 100644 scripts/remove-files.js create mode 100644 smoke-tests/test/npm-replace-global.js diff --git a/.github/workflows/ci-release.yml b/.github/workflows/ci-release.yml index 4abb5d80bf7ad..821d4a37368a3 100644 --- a/.github/workflows/ci-release.yml +++ b/.github/workflows/ci-release.yml @@ -199,6 +199,10 @@ jobs: check_id: ${{ steps.check.outputs.check_id }} smoke-publish: + # This cant be tested on Windows because our node_modules directory + # checks in symlinks which are not supported there. This should be + # fixed somehow, because this means some forms of local development + # are likely broken on Windows as well. name: Smoke Publish - ${{ matrix.platform.name }} - ${{ matrix.node-version }} if: github.repository_owner == 'npm' strategy: @@ -288,12 +292,20 @@ jobs: NPM_VERSION="$(node . --version)-$GITHUB_SHA.0" node . version $NPM_VERSION --ignore-scripts node scripts/publish.js --pack-destination=$RUNNER_TEMP - node . install --global $RUNNER_TEMP/npm-$NPM_VERSION.tgz + export SMOKE_PUBLISH_TARBALL="$RUNNER_TEMP/npm-$NPM_VERSION.tgz" + node . install --global $SMOKE_PUBLISH_TARBALL node . install -w smoke-tests --ignore-scripts --no-audit --no-fund - node scripts/remove-files.js # call installed npm instead of local source since we are testing # the packed tarball that we just installed globally - npm test -w smoke-tests --ignore-scripts + NPM_GLOBAL_VERSION="$(npm --version)" + npm help + if [ "$NPM_GLOBAL_VERSION" == "$NPM_VERSION" ]; then + npm test -w smoke-tests --ignore-scripts + else + echo "global npm is not the correct version for smoke-publish" + echo "found: $NPM_GLOBAL_VERSION, expected: $NPM_VERSION" + exit 1 + fi - name: Conclude Check uses: LouisBrunner/checks-action@v1.3.1 if: steps.check.outputs.check_id && always() diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index 57cb7bceaa75b..d287186cf4853 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -715,8 +715,6 @@ graph LR; npmcli-run-script-->read-package-json-fast; npmcli-run-script-->which; npmcli-smoke-tests-->http-proxy; - npmcli-smoke-tests-->just-extend; - npmcli-smoke-tests-->just-safe-set; npmcli-smoke-tests-->npmcli-eslint-config["@npmcli/eslint-config"]; npmcli-smoke-tests-->npmcli-mock-registry["@npmcli/mock-registry"]; npmcli-smoke-tests-->npmcli-promise-spawn["@npmcli/promise-spawn"]; diff --git a/package-lock.json b/package-lock.json index 185d7f26d4b1f..730a831eee429 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15506,8 +15506,6 @@ "@npmcli/promise-spawn": "^6.0.2", "@npmcli/template-oss": "4.14.1", "http-proxy": "^1.18.1", - "just-extend": "^6.2.0", - "just-safe-set": "^4.2.1", "tap": "^16.3.4", "which": "^3.0.0" }, diff --git a/scripts/remove-files.js b/scripts/remove-files.js deleted file mode 100644 index 75b4385229178..0000000000000 --- a/scripts/remove-files.js +++ /dev/null @@ -1,11 +0,0 @@ -const { join } = require('path') -const { CWD, run, pkg, fs, git } = require('./util.js') - -const main = async () => { - await git.dirty() - for (const p of pkg.files) { - await fs.rimraf(join(CWD, p)) - } -} - -run(main) diff --git a/scripts/template-oss/ci-release.yml b/scripts/template-oss/ci-release.yml index 44b184db8ae81..5170c6e399981 100644 --- a/scripts/template-oss/ci-release.yml +++ b/scripts/template-oss/ci-release.yml @@ -1,6 +1,10 @@ {{> ciRelease}} smoke-publish: + # This cant be tested on Windows because our node_modules directory + # checks in symlinks which are not supported there. This should be + # fixed somehow, because this means some forms of local development + # are likely broken on Windows as well. {{> jobMatrix jobName="Smoke Publish" jobCheck=(obj sha="inputs.check-sha") @@ -14,10 +18,18 @@ NPM_VERSION="$({{ rootNpmPath }} --version)-$GITHUB_SHA.0" {{ rootNpmPath }} version $NPM_VERSION --ignore-scripts node scripts/publish.js --pack-destination=$RUNNER_TEMP - {{ rootNpmPath }} install --global $RUNNER_TEMP/npm-$NPM_VERSION.tgz + export SMOKE_PUBLISH_TARBALL="$RUNNER_TEMP/npm-$NPM_VERSION.tgz" + {{ rootNpmPath }} install --global $SMOKE_PUBLISH_TARBALL {{ rootNpmPath }} install -w smoke-tests --ignore-scripts --no-audit --no-fund - node scripts/remove-files.js # call installed npm instead of local source since we are testing # the packed tarball that we just installed globally - npm test -w smoke-tests --ignore-scripts + NPM_GLOBAL_VERSION="$(npm --version)" + npm help + if [ "$NPM_GLOBAL_VERSION" == "$NPM_VERSION" ]; then + npm test -w smoke-tests --ignore-scripts + else + echo "global npm is not the correct version for smoke-publish" + echo "found: $NPM_GLOBAL_VERSION, expected: $NPM_VERSION" + exit 1 + fi {{> stepChecks jobCheck=true }} diff --git a/smoke-tests/package.json b/smoke-tests/package.json index 0cf40e107d3e5..daf5c5ec83025 100644 --- a/smoke-tests/package.json +++ b/smoke-tests/package.json @@ -23,8 +23,6 @@ "@npmcli/promise-spawn": "^6.0.2", "@npmcli/template-oss": "4.14.1", "http-proxy": "^1.18.1", - "just-extend": "^6.2.0", - "just-safe-set": "^4.2.1", "tap": "^16.3.4", "which": "^3.0.0" }, diff --git a/smoke-tests/tap-snapshots/test/index.js.test.cjs b/smoke-tests/tap-snapshots/test/index.js.test.cjs index 0fae05239d362..5ba8778109226 100644 --- a/smoke-tests/tap-snapshots/test/index.js.test.cjs +++ b/smoke-tests/tap-snapshots/test/index.js.test.cjs @@ -33,7 +33,7 @@ All commands: whoami Specify configs in the ini-formatted file: - {NPM}/{TESTDIR}/project/.npmrc + {NPM}/{TESTDIR}/home/.npmrc or on the command line via: npm --key=value More configuration info: npm help config diff --git a/smoke-tests/test/fixtures/setup.js b/smoke-tests/test/fixtures/setup.js index 180fe02a15383..c17e9fbacee2c 100644 --- a/smoke-tests/test/fixtures/setup.js +++ b/smoke-tests/test/fixtures/setup.js @@ -3,19 +3,23 @@ const { existsSync } = require('fs') const { join, resolve, sep, extname, relative, delimiter } = require('path') const which = require('which') const spawn = require('@npmcli/promise-spawn') -const justExtend = require('just-extend') -const justSet = require('just-safe-set') const MockRegistry = require('@npmcli/mock-registry') -const { Blob } = require('buffer') const http = require('http') const httpProxy = require('http-proxy') -const { SMOKE_PUBLISH_NPM, CI, PATH, Path, TAP_CHILD_ID = '0' } = process.env -const PORT = 12345 + (+TAP_CHILD_ID) +const { SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI, PATH, Path, TAP_CHILD_ID = '0' } = process.env +const PROXY_PORT = 12345 + (+TAP_CHILD_ID) +const HTTP_PROXY = `http://localhost:${PROXY_PORT}` + +const NODE_PATH = process.execPath const CLI_ROOT = resolve(process.cwd(), '..') +const CLI_JS_BIN = join('bin', 'npm-cli.js') +const NPM_PATH = join(CLI_ROOT, CLI_JS_BIN) + +const WINDOWS = process.platform === 'win32' +const GLOBAL_BIN = WINDOWS ? '' : 'bin' +const GLOBAL_NODE_MODULES = join(WINDOWS ? '' : 'lib', 'node_modules') -const set = (obj, ...args) => justSet(obj, ...args) && obj -const merge = (...args) => justExtend(true, ...args) const normalizePath = path => path.replace(/[A-Z]:/, '').replace(/\\/g, '/') const testdirHelper = (obj) => { @@ -31,76 +35,83 @@ const testdirHelper = (obj) => { return obj } -const getSpawnArgs = async () => { +const getNpmRoot = (helpText) => { + return helpText + .split('\n') + .slice(-1)[0] + .match(/^npm@.*?\s(.*)$/) + ?.[1] +} + +const getCleanPaths = async () => { const cliBin = join('bin', 'npm') - const cliJsBin = join('bin', 'npm-cli.js') const npmLinks = await which('npm', { all: true }) const npmPaths = await Promise.all(npmLinks.map(npm => fs.realpath(npm))) const cleanNpmPaths = [...new Set([ CLI_ROOT, join(CLI_ROOT, cliBin), - join(CLI_ROOT, cliJsBin), + join(CLI_ROOT, CLI_JS_BIN), ...npmLinks, ...npmPaths, ...npmPaths.map(n => n.replace(sep + cliBin, '')), - ...npmPaths.map(n => n.replace(sep + cliJsBin, '')), + ...npmPaths.map(n => n.replace(sep + CLI_JS_BIN, '')), ])] - if (SMOKE_PUBLISH_NPM) { - return { - command: ['npm'], - NPM: cleanNpmPaths, - } - } - - return { - command: [process.execPath, join(CLI_ROOT, cliJsBin)], - NODE: process.execPath, + return Object.entries({ + NODE: NODE_PATH, NPM: cleanNpmPaths, - } + }) +} + +const createRegistry = async (t, { debug } = {}) => { + const registry = new MockRegistry({ + tap: t, + registry: 'http://smoke-test-registry.club/', + debug, + strict: true, + }) + + const proxy = httpProxy.createProxyServer({}) + const server = http.createServer((req, res) => proxy.web(req, res, { target: registry.origin })) + await new Promise(res => server.listen(PROXY_PORT, res)) + + t.teardown(() => server.close()) + + return registry } module.exports = async (t, { testdir = {}, debug } = {}) => { + const debugLog = debug || CI ? (...a) => console.error(...a) : () => {} + const cleanPaths = await getCleanPaths() + // setup fixtures const root = t.testdir({ cache: {}, project: { '.npmrc': '' }, - bin: {}, global: { '.npmrc': '' }, + home: { '.npmrc': '' }, ...testdirHelper(testdir), }) const paths = { root, project: join(root, 'project'), global: join(root, 'global'), - userConfig: join(root, 'project', '.npmrc'), + home: join(root, 'home'), + userConfig: join(root, 'home', '.npmrc'), globalConfig: join(root, 'global', '.npmrc'), cache: join(root, 'cache'), - bin: join(root, 'bin'), + globalBin: join(root, 'global', GLOBAL_BIN), + globalNodeModules: join(root, 'global', GLOBAL_NODE_MODULES), } - const registry = new MockRegistry({ - tap: t, - registry: 'http://smoke-test-registry.club/', - debug, - strict: true, - }) - const httpProxyRegistry = `http://localhost:${PORT}` - const proxy = httpProxy.createProxyServer({}) - const server = http.createServer((req, res) => proxy.web(req, res, { target: registry.origin })) - await new Promise(res => server.listen(PORT, res)) - t.teardown(() => server.close()) + const registry = await createRegistry(t, { debug }) // update notifier should never be written t.afterEach((t) => { t.equal(existsSync(join(paths.cache, '_update-notifier-last-checked')), false) }) - const debugLog = debug || CI ? (...a) => console.error(...a) : () => {} - const { command, ...spawnPaths } = await getSpawnArgs({ log: debugLog }) - const cleanPaths = Object.entries(spawnPaths) - const cleanOutput = s => { // sometimes we print normalized paths in snapshots regardless of // platform so replace those first then replace platform style paths @@ -118,7 +129,7 @@ module.exports = async (t, { testdir = {}, debug } = {}) => { } return s .split(relative(CLI_ROOT, t.testdirName)).join('{TESTDIR}') - .split(httpProxyRegistry).join('{REGISTRY}') + .split(HTTP_PROXY).join('{PROXY_REGISTRY}') .split(registry.origin).join('{REGISTRY}') .replace(/\\+/g, '/') .replace(/\r\n/g, '\n') @@ -130,41 +141,23 @@ module.exports = async (t, { testdir = {}, debug } = {}) => { const log = (...a) => debugLog(cleanOutput(a.join(' '))) t.cleanSnapshot = cleanOutput - const npm = async (...args) => { - const defaultFlags = [ - `--registry=${httpProxyRegistry}`, - `--cache=${paths.cache}`, - `--prefix=${paths.project}`, - `--userconfig=${paths.userConfig}`, - `--globalconfig=${paths.globalConfig}`, - '--no-audit', - '--no-update-notifier', - '--loglevel=silly', - '--fetch-retries=0', - ] - const [positionals, flags] = args.reduce((acc, arg) => { - if (arg.startsWith('-')) { - acc[1].push(arg) - } else { - acc[0].push(arg) - } - return acc - }, [[], defaultFlags]) - - const spawnCmd = command[0] - const spawnArgs = [...command.slice(1), ...positionals, ...flags] + const getPath = () => `${paths.globalBin}${delimiter}${Path || PATH}` + const getEnvPath = () => ({ [Path ? 'Path' : 'PATH']: getPath() }) - log(`${spawnCmd} ${spawnArgs.filter(a => !defaultFlags.includes(a)).join(' ')}`) + const baseSpawn = async (spawnCmd, spawnArgs, { cwd = paths.project, env, ...opts } = {}) => { + log(`CWD: ${cwd}`) + log(`${spawnCmd} ${spawnArgs.join(' ')}`) log('-'.repeat(40)) const { stderr, stdout } = await spawn(spawnCmd, spawnArgs, { - cwd: paths.project, + cwd, env: { - HTTP_PROXY: httpProxyRegistry, + ...getEnvPath(), HOME: paths.root, - [Path ? 'Path' : 'PATH']: `${Path || PATH}${delimiter}${paths.bin}`, - COMSPEC: process.env.COMSPEC, + ComSpec: process.env.ComSpec, + ...env, }, + ...opts, }) log(stderr) @@ -172,46 +165,82 @@ module.exports = async (t, { testdir = {}, debug } = {}) => { log(stdout) log('='.repeat(40)) - return cleanOutput(stdout) + return stdout } + const baseNpm = async ({ cwd, cmd, argv = [], proxy = true, ...opts } = {}, ...args) => { + const isGlobal = args.some(a => ['-g', '--global', '--global=true'].includes(a)) + + const defaultFlags = [ + proxy ? `--registry=${HTTP_PROXY}` : null, + `--cache=${paths.cache}`, + `--prefix=${isGlobal ? paths.global : cwd}`, + `--userconfig=${paths.userConfig}`, + `--globalconfig=${paths.globalConfig}`, + '--no-audit', + '--no-update-notifier', + '--loglevel=silly', + '--fetch-retries=0', + ].filter(Boolean) + + const cliArgv = args.reduce((acc, arg) => { + if (arg.startsWith('-')) { + acc[1].push(arg) + } else { + acc[0].push(arg) + } + return acc + }, [[], defaultFlags]).reduce((acc, i) => acc.concat(i), []) + + return baseSpawn(cmd, [...argv, ...cliArgv], { + cwd, + env: proxy ? { HTTP_PROXY } : {}, + ...opts, + }) + } + + const npmLocal = (...args) => baseNpm({ + cwd: CLI_ROOT, + cmd: process.execPath, + argv: ['.'], + proxy: false, + }, ...args) + + const npmPath = (...args) => baseNpm({ + cwd: paths.project, + cmd: 'npm', + shell: true, + }, ...args) + + const npm = (...args) => baseNpm({ + cwd: paths.project, + cmd: process.execPath, + argv: [NPM_PATH], + }, ...args) + // helpers for reading/writing files and their source const readFile = async (f) => { const file = await fs.readFile(join(paths.project, f), 'utf-8') return extname(f) === '.json' ? JSON.parse(file) : file } - // Returns a recurisve list of relative file paths in the testdir root - // will also follow symlinks and print their relative paths - const tree = async (rootDir = paths.project, dir = rootDir) => { - const results = {} - for (const item of await fs.readdir(dir)) { - const itemPath = join(dir, item) - const relPath = relative(rootDir, itemPath) - const stat = await fs.lstat(itemPath) - - if (stat.isSymbolicLink()) { - const realpath = await fs.realpath(itemPath) - merge(results, await tree(rootDir, realpath)) - } else if (stat.isDirectory()) { - merge(results, await tree(rootDir, itemPath)) - } else { - const raw = await readFile(relPath) - const content = typeof raw === 'string' ? `${new Blob([raw]).size} bytes` : raw - merge(results, set({}, relPath.split(sep), content)) - } - } - return results - } - return { - npm, + npmPath, + npmLocal: SMOKE_PUBLISH_NPM ? async () => { + throw new Error('npmLocal cannot be called during smoke-publish') + } : npmLocal, + npm: SMOKE_PUBLISH_NPM ? npmPath : npm, + spawn: baseSpawn, readFile, - tree, + getPath, paths, registry, - isSmokePublish: !!SMOKE_PUBLISH_NPM, } } module.exports.testdir = testdirHelper +module.exports.getNpmRoot = getNpmRoot +module.exports.CLI_ROOT = CLI_ROOT +module.exports.WINDOWS = WINDOWS +module.exports.SMOKE_PUBLISH = !!SMOKE_PUBLISH_NPM +module.exports.SMOKE_PUBLISH_TARBALL = SMOKE_PUBLISH_TARBALL diff --git a/smoke-tests/test/index.js b/smoke-tests/test/index.js index 152bab15666f2..fc005dc7cd8f2 100644 --- a/smoke-tests/test/index.js +++ b/smoke-tests/test/index.js @@ -3,7 +3,7 @@ const t = require('tap') const setup = require('./fixtures/setup.js') t.test('basic', async t => { - const { registry, npm, isSmokePublish, readFile, paths } = await setup(t, { + const { registry, npm, npmPath, npmLocal, readFile, paths } = await setup(t, { testdir: { packages: { 'abbrev-1.0.4': { @@ -43,10 +43,32 @@ t.test('basic', async t => { t.equal(pkg.version, '1.0.0', 'should have expected generated version') }) + await t.test('npm root', async t => { + const npmRoot = await npm('help').then(setup.getNpmRoot) + + if (setup.SMOKE_PUBLISH) { + const globalNpmRoot = await npmPath('help').then(setup.getNpmRoot) + t.rejects(npmLocal('help'), 'npm local rejects during smoke publish') + t.not(npmRoot, setup.CLI_ROOT, 'npm root is not the local source dir') + t.equal( + npmRoot, + globalNpmRoot, + 'during smoke publish, npm root and global root are equal' + ) + } else { + t.equal( + await npmLocal('help').then(setup.getNpmRoot), + setup.CLI_ROOT, + 'npm local root is the local source dir' + ) + t.equal(npmRoot, setup.CLI_ROOT, 'npm root is the local source dir') + } + }) + await t.test('npm --version', async t => { const v = await npm('--version') - if (isSmokePublish) { + if (setup.SMOKE_PUBLISH) { t.match(v.trim(), /-[0-9a-f]{40}\.\d$/, 'must have a git version') } else { t.match(v.trim(), /^\d+\.\d+\.\d+/, 'has a version') diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js new file mode 100644 index 0000000000000..ee1ac5d89a778 --- /dev/null +++ b/smoke-tests/test/npm-replace-global.js @@ -0,0 +1,82 @@ + +const t = require('tap') +const { join, dirname, basename, extname } = require('path') +const fs = require('fs/promises') +const _which = require('which') +const setup = require('./fixtures/setup.js') + +const which = async (cmd, opts) => { + const path = await _which(cmd, { nothrow: true, ...opts }) + return path ? join(dirname(path), basename(path, extname(path))) : null +} + +t.test('npm replace global', async t => { + const { + npm, + npmLocal, + npmPath, + getPath, + paths: { root, globalBin, globalNodeModules }, + } = await setup(t, { + testdir: { + project: { + 'package.json': { name: 'npm', version: '999.999.999' }, + }, + }, + }) + + const getPaths = async () => { + const binContents = await fs.readdir(globalBin).then(results => results + .filter(p => p !== '.npmrc' && p !== 'node_modules') + .map(p => basename(p, extname(p))) + .reduce((set, p) => set.add(p), new Set())) + + return { + npmRoot: await npmPath('help').then(setup.getNpmRoot), + pathNpm: await which('npm', { path: getPath(), nothrow: true }), + globalNpm: await which('npm', { nothrow: true }), + pathNpx: await which('npx', { path: getPath(), nothrow: true }), + globalNpx: await which('npx', { nothrow: true }), + binContents: [...binContents], + nodeModulesContents: await fs.readdir(join(globalNodeModules, 'npm')), + } + } + + const tarball = setup.SMOKE_PUBLISH_TARBALL ?? + await npmLocal('pack', `--pack-destination=${root}`).then(r => join(root, r)) + + await npm('install', tarball, '--global') + + t.equal( + await fs.realpath(join(globalBin, 'npm')), + setup.WINDOWS ? join(globalBin, 'npm') : join(globalNodeModules, 'npm/bin/npm-cli.js'), + 'npm realpath is in the testdir' + ) + t.equal( + await fs.realpath(join(globalBin, 'npx')), + setup.WINDOWS ? join(globalBin, 'npx') : join(globalNodeModules, 'npm/bin/npx-cli.js'), + 'npx realpath is in the testdir' + ) + + const prePaths = await getPaths() + t.equal(prePaths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') + t.equal(prePaths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') + t.equal(prePaths.pathNpx, join(globalBin, 'npx'), 'npx bin is in the testdir') + t.not(prePaths.pathNpm, prePaths.globalNpm, 'npm bin is not the same as the global one') + t.not(prePaths.pathNpx, prePaths.globalNpx, 'npm bin is not the same as the global one') + t.match(prePaths.binContents, ['npm', 'npx'], 'bin has npm and npx') + t.ok(prePaths.nodeModulesContents.length > 1, 'node modules has npm contents') + t.ok(prePaths.nodeModulesContents.includes('node_modules'), 'npm has its node_modules') + + await npmPath('pack') + await npmPath('install', 'npm-999.999.999.tgz', '--global') + + const postPaths = await getPaths() + t.not(prePaths.npmRoot, postPaths.npmRoot, 'npm roots are different') + t.equal(postPaths.pathNpm, postPaths.globalNpm, 'npm bin is the same as the global one') + t.equal(postPaths.pathNpx, postPaths.globalNpx, 'npx bin is the same as the global one') + t.equal(postPaths.pathNpm, prePaths.globalNpm, 'after install npm bin is same as previous global') + t.equal(postPaths.pathNpx, prePaths.globalNpx, 'after install npx bin is same as previous global') + t.strictSame(postPaths.binContents, [], 'bin is empty') + t.strictSame(postPaths.nodeModulesContents, ['package.json'], 'contents is only package.json') +})