From 0b1ae128e471c64ccf48c6761919cd3bd995b1bb Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 09:54:10 -0700 Subject: [PATCH 01/10] chore: create smoke-publish-test.sh scripts This makes it easier to debug this test locally. Previously this was avoided because the test will install npm globally and prune deps which can be disruptive to a local checkout. This is somewhat mitigated now with some cleanup and better messaging when run locally. --- .github/workflows/ci-release.yml | 23 +------ docs/bin/build.js | 14 ---- mock-registry/lib/index.js | 22 ++++-- scripts/publish.js | 28 +++++--- scripts/smoke-publish-test.sh | 92 +++++++++++++++++++++++++ scripts/template-oss/ci-release-yml.hbs | 23 +------ scripts/util.js | 2 +- smoke-tests/test/fixtures/setup.js | 2 + smoke-tests/test/npm-replace-global.js | 58 +++++++++------- 9 files changed, 166 insertions(+), 98 deletions(-) create mode 100755 scripts/smoke-publish-test.sh diff --git a/.github/workflows/ci-release.yml b/.github/workflows/ci-release.yml index d2b3c5291ce1e..60e4e4144e207 100644 --- a/.github/workflows/ci-release.yml +++ b/.github/workflows/ci-release.yml @@ -217,27 +217,8 @@ jobs: run: node scripts/git-dirty.js - name: Reset Deps run: node scripts/resetdeps.js - - name: Pack - env: - SMOKE_PUBLISH_NPM: 1 - run: | - NPM_VERSION="$(node . --version)-$GITHUB_SHA.0" - node . version $NPM_VERSION --ignore-scripts - node scripts/publish.js --pack-destination=$RUNNER_TEMP - 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 - # call installed npm instead of local source since we are testing - # the packed tarball that we just installed globally - 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: Smoke Publish + run: ./scripts/smoke-publish-test.sh - name: Conclude Check uses: LouisBrunner/checks-action@v1.6.0 if: always() diff --git a/docs/bin/build.js b/docs/bin/build.js index c25fd76ed7e7f..602596bc2d494 100644 --- a/docs/bin/build.js +++ b/docs/bin/build.js @@ -1,17 +1,3 @@ -if ( - process.env.SMOKE_PUBLISH_NPM && - !require('semver').satisfies(process.version, require('../package.json').engines.node) -) { - // The docs tooling is kept in sync between releases and dependencies that are not compatible - // with the lower bound of npm@8 engines are used. When we run the SMOKE_PUBLISH_NPM we are - // testing that npm is able to pack and install itself locally and then run its own smoke tests. - // Packing will run this script automatically so in the cases where the node version is - // not compatible, it is ok to bail on this script since the generated docs are not used in - // the smoke tests. - console.log(`Skipping docs build due to SMOKE_PUBLISH_NPM and ${process.version}`) - return -} - const run = require('../lib/build.js') const { paths } = require('../lib/index') diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 03787ea7cd4e6..912f324a77fb8 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -4,6 +4,17 @@ const npa = require('npm-package-arg') const Nock = require('nock') const stringify = require('json-stringify-safe') +const logReq = (req, ...keys) => { + const obj = JSON.parse(stringify(req)) + const res = {} + for (const [k, v] of Object.entries(obj)) { + if (!keys.includes(k)) { + res[k] = v + } + } + return stringify(res, null, 2) +} + class MockRegistry { #tap #nock @@ -40,7 +51,8 @@ class MockRegistry { // mocked with a 404, 500, etc. // XXX: this is opt-in currently because it breaks some existing CLI // tests. We should work towards making this the default for all tests. - t.fail(`Unmatched request: ${stringify(req, null, 2)}`) + t.comment(logReq(req, 'interceptors', 'socket', 'response', '_events')) + t.fail(`Unmatched request: ${req.method} ${req.path}`) } } @@ -357,7 +369,7 @@ class MockRegistry { }) } - async package ({ manifest, times = 1, query, tarballs }) { + async package ({ manifest, times = 1, query, tarballs, tarballTimes = 1 }) { let nock = this.nock const spec = npa(manifest.name) nock = nock.get(this.fullPath(`/${spec.escapedName}`)).times(times) @@ -368,17 +380,17 @@ class MockRegistry { if (tarballs) { for (const [version, tarball] of Object.entries(tarballs)) { const m = manifest.versions[version] - nock = await this.tarball({ manifest: m, tarball }) + nock = await this.tarball({ manifest: m, tarball, times: tarballTimes }) } } this.nock = nock } - async tarball ({ manifest, tarball }) { + async tarball ({ manifest, tarball, times = 1 }) { const nock = this.nock const dist = new URL(manifest.dist.tarball) const tar = await pacote.tarball(tarball, { Arborist }) - nock.get(this.fullPath(dist.pathname)).reply(200, tar) + nock.get(this.fullPath(dist.pathname)).times(times).reply(200, tar) return nock } diff --git a/scripts/publish.js b/scripts/publish.js index f27b07fb8e658..a28bfd849120c 100644 --- a/scripts/publish.js +++ b/scripts/publish.js @@ -72,8 +72,9 @@ const getPublishes = async ({ force }) => { } const main = async (opts) => { - const packOnly = opts.pack || opts.packDestination - const publishes = await getPublishes({ force: packOnly }) + const { isLocal, smokePublish, packDestination } = opts + const isPack = !!packDestination + const publishes = await getPublishes({ force: isPack }) if (!publishes.length) { throw new Error( @@ -88,12 +89,12 @@ const main = async (opts) => { } const confirmMessage = [ - `Ready to ${packOnly ? 'pack' : 'publish'} the following packages:`, + `Ready to ${isPack ? 'pack' : 'publish'} the following packages:`, table.toString(), - packOnly ? null : 'Ok to proceed? ', + isPack ? null : 'Ok to proceed? ', ].filter(Boolean).join('\n') - if (packOnly) { + if (isPack) { log.info(confirmMessage) } else { const confirm = await read({ prompt: confirmMessage, default: 'y' }) @@ -116,21 +117,26 @@ const main = async (opts) => { await npm('prune', '--omit=dev', '--no-save', '--no-audit', '--no-fund') await npm('install', '-w', 'docs', '--ignore-scripts', '--no-audit', '--no-fund') - await git.dirty() + if (isLocal && smokePublish) { + log.info(`Skipping git dirty check due to local smoke publish test being run`) + } else { + await git.dirty() + } for (const publish of publishes) { const workspace = publish.workspace && `--workspace=${publish.name}` - if (packOnly) { + const publishPkg = (...args) => npm('publish', workspace, `--tag=${publish.tag}`, ...args) + if (isPack) { await npm( 'pack', workspace, opts.packDestination && `--pack-destination=${opts.packDestination}` ) + if (smokePublish) { + await publishPkg('--dry-run') + } } else { - await npm( - 'publish', - workspace, - `--tag=${publish.tag}`, + await publishPkg( opts.dryRun && '--dry-run', opts.otp && `--otp=${opts.otp === 'op' ? await op() : opts.otp}` ) diff --git a/scripts/smoke-publish-test.sh b/scripts/smoke-publish-test.sh new file mode 100755 index 0000000000000..871ff6700e232 --- /dev/null +++ b/scripts/smoke-publish-test.sh @@ -0,0 +1,92 @@ + #!/usr/bin/env bash + +set -eo pipefail + +IS_LOCAL="false" +IS_CI="true" + +if [ -z "$CI" ]; then + echo "Running locally will overwrite your globally installed npm." + GITHUB_SHA=$(git rev-parse HEAD) + RUNNER_TEMP=$(mktemp -d) + IS_LOCAL="true" + IS_CI="false" +fi + +if [ -z "$GITHUB_SHA" ]; then + echo "Error: GITHUB_SHA is required" + exit 1 +fi + +if [ -z "$RUNNER_TEMP" ]; then + echo "Error: RUNNER_TEMP is required" + exit 1 +fi + +ORIGINAL_GLOBAL_NPM_VERSION=$(npm --version) +if [ ${#ORIGINAL_GLOBAL_NPM_VERSION} -gt 40 ]; then + echo "Error: Global npm version already contains a git SHA ${ORIGINAL_GLOBAL_NPM_VERSION}" + exit 1 +fi + +ORIGINAL_LOCAL_NPM_VERSION=$(node . --version) +if [ ${#ORIGINAL_LOCAL_NPM_VERSION} -gt 40 ]; then + echo "Error: Local npm version already contains a git SHA ${ORIGINAL_LOCAL_NPM_VERSION}" + exit 1 +fi +NPM_VERSION="$ORIGINAL_LOCAL_NPM_VERSION-$GITHUB_SHA.0" + +# Only cleanup locally +if [ "$IS_LOCAL" == "true" ]; then + function cleanup { + npm pkg set version=$ORIGINAL_LOCAL_NPM_VERSION + node scripts/resetdeps.js + if [ "$(git rev-parse HEAD)" != "$GITHUB_SHA" ]; then + echo "===================================" + echo "===================================" + echo "HEAD is on a different commit." + echo "===================================" + echo "===================================" + fi + if [ "$(npm --version)" == "$NPM_VERSION" ]; then + echo "===================================" + echo "===================================" + echo "Global npm version has changed to $NPM_VERSION" + echo "Run the following to change it back" + echo "npm install npm@$ORIGINAL_GLOBAL_NPM_VERSION -g" + echo "===================================" + echo "===================================" + fi + } + trap cleanup EXIT +fi + +# Version the local source of npm with the current git sha and +# and pack and install it globally the same way we would if we +# were publishing it to the registry. The only difference is in the +# publish.js script which will only pack and not publish +node . version $NPM_VERSION --ignore-scripts --git-tag-version="$IS_CI" +node scripts/publish.js --pack-destination=$RUNNER_TEMP --smoke-publish=true --is-local="$IS_LOCAL" +NPM_TARBALL="$RUNNER_TEMP/npm-$NPM_VERSION.tgz" +node . install --global $NPM_TARBALL + +# Only run the tests if we are sure we have the right version +# otherwise the tests being run are pointless +NPM_GLOBAL_VERSION="$(npm --version)" +if [ "$NPM_GLOBAL_VERSION" != "$NPM_VERSION" ]; then + echo "global npm is not the correct version for smoke-publish" + echo "found: $NPM_GLOBAL_VERSION, expected: $NPM_VERSION" + exit 1 +fi + +# Install dev deps only for smoke tests so they can be run +node . install -w smoke-tests --ignore-scripts --no-audit --no-fund +# Run smoke tests with env vars so it uses the globally installed tarball we +# just packed/installed. The tacked on args at the end are only used for +# debugging locally when we want to pass args to the smoke-tests to limit the +# files being run or grep a test, etc. Also now set CI=true so we get more +# debug output in our tap tests +CI="true" SMOKE_PUBLISH_NPM="1" SMOKE_PUBLISH_TARBALL="$NPM_TARBALL" npm test \ + -w smoke-tests \ + --ignore-scripts \ + -- -Rtap "$@" diff --git a/scripts/template-oss/ci-release-yml.hbs b/scripts/template-oss/ci-release-yml.hbs index c3baff23c906b..9078f02c3429f 100644 --- a/scripts/template-oss/ci-release-yml.hbs +++ b/scripts/template-oss/ci-release-yml.hbs @@ -11,27 +11,8 @@ jobCreateCheck=(obj sha="${{ inputs.check-sha }}") windowsCI=false }} - - name: Pack - env: - SMOKE_PUBLISH_NPM: 1 - run: | - NPM_VERSION="$({{ rootNpmPath }} --version)-$GITHUB_SHA.0" - {{ rootNpmPath }} version $NPM_VERSION --ignore-scripts - node scripts/publish.js --pack-destination=$RUNNER_TEMP - 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 - # call installed npm instead of local source since we are testing - # the packed tarball that we just installed globally - 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: Smoke Publish + run: ./scripts/smoke-publish-test.sh - name: Conclude Check uses: LouisBrunner/checks-action@v1.6.0 if: always() diff --git a/scripts/util.js b/scripts/util.js index c842bd26004f3..9c9973914b1f3 100644 --- a/scripts/util.js +++ b/scripts/util.js @@ -132,7 +132,7 @@ git.dirty = () => npmGit.isClean({ cwd: CWD }).then(async r => { return 'git clean' } await git('status', '--porcelain=v1', '-uno') - await git('diff') + await git('--no-pager', 'diff') throw new Error('git dirty') }) diff --git a/smoke-tests/test/fixtures/setup.js b/smoke-tests/test/fixtures/setup.js index 91e0ddec2415f..7f35ccb643e6d 100644 --- a/smoke-tests/test/fixtures/setup.js +++ b/smoke-tests/test/fixtures/setup.js @@ -75,6 +75,8 @@ const getCleanPaths = async () => { module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy = false } = {}) => { const debugLog = debug || CI ? (...a) => t.comment(...a) : () => {} + debugLog({ SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI }) + const cleanPaths = await getCleanPaths() // setup fixtures diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 12364e6899d9f..e5aaca3d0e5cc 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -94,7 +94,6 @@ t.test('pack and replace global self', async t => { }) t.test('publish and replace global self', async t => { - let publishedPackument = null const pkg = require('../../package.json') const { name, version } = pkg @@ -114,39 +113,42 @@ t.test('publish and replace global self', async t => { }, }) - const npmPackage = async ({ manifest, ...opts } = {}) => { + const mockNpmPackage = async ({ manifest, ...opts } = {}) => { await registry.package({ manifest: registry.manifest({ name, ...manifest }), ...opts, }) - } - - const npmInstall = async (useNpm) => { - await npmPackage({ - manifest: { packuments: [publishedPackument] }, - tarballs: { [version]: tarball }, - times: 3, - }) await fs.rm(cache, { recursive: true, force: true }) - await useNpm('install', 'npm@latest', '--global') - return getPaths() } - const tarball = await npmLocalTarball() - - if (setup.SMOKE_PUBLISH) { - await npmPackage() - } - registry.nock.put('/npm', body => { - if (body._id === 'npm' && body.versions[version]) { - publishedPackument = body.versions[version] - return true + const mockPublish = (() => { + let publishedPackument = null + registry.nock.put('/npm', body => { + if (body._id === 'npm' && body.versions[version]) { + publishedPackument = body.versions[version] + return true + } + return false + }).reply(201, {}) + return { + get packument () { + return publishedPackument + }, } - return false - }).reply(201, {}) + })() + await mockNpmPackage() await npmLocal('publish', { proxy: true, force: true }) - const paths = await npmInstall(npm) + const tarball = await npmLocalTarball() + + await mockNpmPackage({ + manifest: { packuments: [mockPublish.packument] }, + tarballs: { [version]: tarball }, + times: 3, + }) + await npm('install', 'npm@latest', '--global') + const paths = await getPaths() + t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') t.equal(paths.pathNpx, join(globalBin, 'npx'), 'npx bin is in the testdir') @@ -159,7 +161,13 @@ t.test('publish and replace global self', async t => { 'bin has npm and npx' ) - t.strictSame(await npmInstall(npmPath), paths) + await mockNpmPackage({ + manifest: { packuments: [mockPublish.packument] }, + tarballs: { [version]: tarball }, + times: 3, + }) + await npmPath('install', 'npm@latest', '--global') + t.strictSame(await getPaths(), paths) }) t.test('fail when updating with lazy require', async t => { From 1d24db047fed05413dd7a7bf0d92c55211364a4f Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 15:07:14 -0700 Subject: [PATCH 02/10] tarball times --- smoke-tests/test/npm-replace-global.js | 63 +++++++++++--------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index e5aaca3d0e5cc..758d5ba7e8230 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -94,9 +94,6 @@ t.test('pack and replace global self', async t => { }) t.test('publish and replace global self', async t => { - const pkg = require('../../package.json') - const { name, version } = pkg - const { npm, npmPath, @@ -113,42 +110,42 @@ t.test('publish and replace global self', async t => { }, }) - const mockNpmPackage = async ({ manifest, ...opts } = {}) => { + let publishedPackument = null + const { name, version } = require('../../package.json') + + const npmPackage = async ({ manifest, ...opts } = {}) => { await registry.package({ manifest: registry.manifest({ name, ...manifest }), ...opts, }) - await fs.rm(cache, { recursive: true, force: true }) } - const mockPublish = (() => { - let publishedPackument = null - registry.nock.put('/npm', body => { - if (body._id === 'npm' && body.versions[version]) { - publishedPackument = body.versions[version] - return true - } - return false - }).reply(201, {}) - return { - get packument () { - return publishedPackument - }, - } - })() - await mockNpmPackage() - await npmLocal('publish', { proxy: true, force: true }) + const npmInstall = async (useNpm) => { + await npmPackage({ + manifest: { packuments: [publishedPackument] }, + tarballs: { [version]: tarball }, + times: 3, + }) + await fs.rm(cache, { recursive: true, force: true }) + await useNpm('install', 'npm@latest', '--global') + return getPaths() + } const tarball = await npmLocalTarball() - await mockNpmPackage({ - manifest: { packuments: [mockPublish.packument] }, - tarballs: { [version]: tarball }, - times: 3, - }) - await npm('install', 'npm@latest', '--global') - const paths = await getPaths() + if (setup.SMOKE_PUBLISH) { + await npmPackage() + } + registry.nock.put('/npm', body => { + if (body._id === 'npm' && body.versions[version]) { + publishedPackument = body.versions[version] + return true + } + return false + }).reply(201, {}) + await npmLocal('publish', { proxy: true, force: true }) + const paths = await npmInstall(npm) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') t.equal(paths.pathNpx, join(globalBin, 'npx'), 'npx bin is in the testdir') @@ -161,13 +158,7 @@ t.test('publish and replace global self', async t => { 'bin has npm and npx' ) - await mockNpmPackage({ - manifest: { packuments: [mockPublish.packument] }, - tarballs: { [version]: tarball }, - times: 3, - }) - await npmPath('install', 'npm@latest', '--global') - t.strictSame(await getPaths(), paths) + t.strictSame(await npmInstall(npmPath), paths) }) t.test('fail when updating with lazy require', async t => { From 405353ea4349727ce80255d1945aa08596970d09 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 15:21:17 -0700 Subject: [PATCH 03/10] more? --- smoke-tests/test/npm-replace-global.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 758d5ba7e8230..b12f1b2b1c7ca 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -145,6 +145,9 @@ t.test('publish and replace global self', async t => { }).reply(201, {}) await npmLocal('publish', { proxy: true, force: true }) + if (setup.SMOKE_PUBLISH) { + await npmPackage() + } const paths = await npmInstall(npm) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') From a747bad968fa50251c20db1a39e63d30315a6ba8 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 15:38:19 -0700 Subject: [PATCH 04/10] more --- smoke-tests/test/npm-replace-global.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index b12f1b2b1c7ca..881fe3c78e1f5 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -146,7 +146,7 @@ t.test('publish and replace global self', async t => { await npmLocal('publish', { proxy: true, force: true }) if (setup.SMOKE_PUBLISH) { - await npmPackage() + await npmPackage({ tarballTimes: 2 }) } const paths = await npmInstall(npm) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') From ffcac2677d9d87b4ece0115ec35e23e4d5f58d8d Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 15:43:30 -0700 Subject: [PATCH 05/10] try this --- smoke-tests/test/npm-replace-global.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 881fe3c78e1f5..995430cff9eba 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -133,9 +133,9 @@ t.test('publish and replace global self', async t => { const tarball = await npmLocalTarball() - if (setup.SMOKE_PUBLISH) { - await npmPackage() - } + // if (setup.SMOKE_PUBLISH) { + // await npmPackage() + // } registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] @@ -145,9 +145,9 @@ t.test('publish and replace global self', async t => { }).reply(201, {}) await npmLocal('publish', { proxy: true, force: true }) - if (setup.SMOKE_PUBLISH) { - await npmPackage({ tarballTimes: 2 }) - } + // if (setup.SMOKE_PUBLISH) { + // await npmPackage({ tarballTimes: 2 }) + // } const paths = await npmInstall(npm) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') From fd0f31457dcaf71306250381de8f4655ef7e89fc Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 15:49:53 -0700 Subject: [PATCH 06/10] ugh --- smoke-tests/test/npm-replace-global.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 995430cff9eba..f11f191d064be 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -120,11 +120,12 @@ t.test('publish and replace global self', async t => { }) } - const npmInstall = async (useNpm) => { + const npmInstall = async (useNpm, opts) => { await npmPackage({ manifest: { packuments: [publishedPackument] }, tarballs: { [version]: tarball }, times: 3, + ...opts, }) await fs.rm(cache, { recursive: true, force: true }) await useNpm('install', 'npm@latest', '--global') @@ -133,9 +134,6 @@ t.test('publish and replace global self', async t => { const tarball = await npmLocalTarball() - // if (setup.SMOKE_PUBLISH) { - // await npmPackage() - // } registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] @@ -145,10 +143,9 @@ t.test('publish and replace global self', async t => { }).reply(201, {}) await npmLocal('publish', { proxy: true, force: true }) - // if (setup.SMOKE_PUBLISH) { - // await npmPackage({ tarballTimes: 2 }) - // } - const paths = await npmInstall(npm) + const paths = await npmInstall(npm, setup.SMOKE_PUBLISH ? { + tarballTimes: 2 + } : {}) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') t.equal(paths.pathNpx, join(globalBin, 'npx'), 'npx bin is in the testdir') From 1a3577fc19451d2be2c6211f404d947b7fe16157 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 15:55:12 -0700 Subject: [PATCH 07/10] need the get on smoke publish --- smoke-tests/test/npm-replace-global.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index f11f191d064be..08a3e5c8a1e31 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -134,6 +134,9 @@ t.test('publish and replace global self', async t => { const tarball = await npmLocalTarball() + if (setup.SMOKE_PUBLISH) { + await npmPackage() + } registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] From d2803f376cbe0541d767142f22bc326a2699cd8c Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 19:06:50 -0700 Subject: [PATCH 08/10] always stream smoke test logs --- smoke-tests/test/fixtures/setup.js | 14 +++----------- smoke-tests/test/npm-replace-global.js | 4 +--- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/smoke-tests/test/fixtures/setup.js b/smoke-tests/test/fixtures/setup.js index 7f35ccb643e6d..e4f7dece22d6e 100644 --- a/smoke-tests/test/fixtures/setup.js +++ b/smoke-tests/test/fixtures/setup.js @@ -172,19 +172,11 @@ module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy }) // In debug mode, stream stdout and stderr to console so we can debug hanging processes - if (debug) { - p.process.stdout.on('data', (c) => log('STDOUT: ' + c.toString().trim())) - p.process.stderr.on('data', (c) => log('STDERR: ' + c.toString().trim())) - } + p.process.stdout.on('data', (c) => log(c.toString().trim())) + p.process.stderr.on('data', (c) => log(c.toString().trim())) const { stdout, stderr } = await p - // If not in debug mode, print full stderr and stdout contents separately - if (!debug) { - log(stderr) - log('-'.repeat(40)) - log(stdout) - log('='.repeat(40)) - } + log('='.repeat(40)) return { stderr, stdout } } diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 08a3e5c8a1e31..3b3d6d5e672ef 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -146,9 +146,7 @@ t.test('publish and replace global self', async t => { }).reply(201, {}) await npmLocal('publish', { proxy: true, force: true }) - const paths = await npmInstall(npm, setup.SMOKE_PUBLISH ? { - tarballTimes: 2 - } : {}) + const paths = await npmInstall(npm) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') t.equal(paths.pathNpx, join(globalBin, 'npx'), 'npx bin is in the testdir') From f15d85216e697b794bb7681e8d51bf3ef6ba1570 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 21:58:56 -0700 Subject: [PATCH 09/10] comment --- smoke-tests/test/npm-replace-global.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 3b3d6d5e672ef..c316369d296e7 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -146,6 +146,8 @@ t.test('publish and replace global self', async t => { }).reply(201, {}) await npmLocal('publish', { proxy: true, force: true }) + t.comment(JSON.stringify(publishedPackument, null, 2)) + const paths = await npmInstall(npm) t.equal(paths.npmRoot, join(globalNodeModules, 'npm'), 'npm root is in the testdir') t.equal(paths.pathNpm, join(globalBin, 'npm'), 'npm bin is in the testdir') From 7228ee81326dedf82f58ba228264be75009c2365 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 8 May 2024 22:06:50 -0700 Subject: [PATCH 10/10] dont run smoke publish on mac ci --- .github/workflows/ci-release.yml | 19 +------------------ mock-registry/lib/index.js | 8 ++++---- scripts/smoke-publish-test.sh | 4 ++-- scripts/template-oss/ci-release-yml.hbs | 3 ++- smoke-tests/test/fixtures/setup.js | 10 +++++----- smoke-tests/test/npm-replace-global.js | 3 +-- 6 files changed, 15 insertions(+), 32 deletions(-) diff --git a/.github/workflows/ci-release.yml b/.github/workflows/ci-release.yml index 60e4e4144e207..bd6cbf8ab4f1d 100644 --- a/.github/workflows/ci-release.yml +++ b/.github/workflows/ci-release.yml @@ -162,29 +162,12 @@ jobs: - name: Linux os: ubuntu-latest shell: bash - - name: macOS - os: macos-latest - shell: bash - - name: macOS - os: macos-13 - shell: bash node-version: - 18.17.0 - 18.x - 20.5.0 - 20.x - 22.x - exclude: - - platform: { name: macOS, os: macos-13, shell: bash } - node-version: 18.17.0 - - platform: { name: macOS, os: macos-13, shell: bash } - node-version: 18.x - - platform: { name: macOS, os: macos-13, shell: bash } - node-version: 20.5.0 - - platform: { name: macOS, os: macos-13, shell: bash } - node-version: 20.x - - platform: { name: macOS, os: macos-13, shell: bash } - node-version: 22.x runs-on: ${{ matrix.platform.os }} defaults: run: @@ -221,7 +204,7 @@ jobs: run: ./scripts/smoke-publish-test.sh - name: Conclude Check uses: LouisBrunner/checks-action@v1.6.0 - if: always() + if: steps.create-check.outputs.check-id && always() with: token: ${{ secrets.GITHUB_TOKEN }} conclusion: ${{ job.status }} diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 912f324a77fb8..727b37e675fc3 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -369,7 +369,7 @@ class MockRegistry { }) } - async package ({ manifest, times = 1, query, tarballs, tarballTimes = 1 }) { + async package ({ manifest, times = 1, query, tarballs }) { let nock = this.nock const spec = npa(manifest.name) nock = nock.get(this.fullPath(`/${spec.escapedName}`)).times(times) @@ -380,17 +380,17 @@ class MockRegistry { if (tarballs) { for (const [version, tarball] of Object.entries(tarballs)) { const m = manifest.versions[version] - nock = await this.tarball({ manifest: m, tarball, times: tarballTimes }) + nock = await this.tarball({ manifest: m, tarball }) } } this.nock = nock } - async tarball ({ manifest, tarball, times = 1 }) { + async tarball ({ manifest, tarball }) { const nock = this.nock const dist = new URL(manifest.dist.tarball) const tar = await pacote.tarball(tarball, { Arborist }) - nock.get(this.fullPath(dist.pathname)).times(times).reply(200, tar) + nock.get(this.fullPath(dist.pathname)).reply(200, tar) return nock } diff --git a/scripts/smoke-publish-test.sh b/scripts/smoke-publish-test.sh index 871ff6700e232..1d08a0adf2bc8 100755 --- a/scripts/smoke-publish-test.sh +++ b/scripts/smoke-publish-test.sh @@ -86,7 +86,7 @@ node . install -w smoke-tests --ignore-scripts --no-audit --no-fund # debugging locally when we want to pass args to the smoke-tests to limit the # files being run or grep a test, etc. Also now set CI=true so we get more # debug output in our tap tests -CI="true" SMOKE_PUBLISH_NPM="1" SMOKE_PUBLISH_TARBALL="$NPM_TARBALL" npm test \ +CI="true" SMOKE_PUBLISH_TARBALL="$NPM_TARBALL" npm test \ -w smoke-tests \ --ignore-scripts \ - -- -Rtap "$@" + -- "$@" diff --git a/scripts/template-oss/ci-release-yml.hbs b/scripts/template-oss/ci-release-yml.hbs index 9078f02c3429f..8ff869812a331 100644 --- a/scripts/template-oss/ci-release-yml.hbs +++ b/scripts/template-oss/ci-release-yml.hbs @@ -10,12 +10,13 @@ jobCheckout=(obj ref="${{ inputs.ref }}") jobCreateCheck=(obj sha="${{ inputs.check-sha }}") windowsCI=false + macCI=false }} - name: Smoke Publish run: ./scripts/smoke-publish-test.sh - name: Conclude Check uses: LouisBrunner/checks-action@v1.6.0 - if: always() + if: steps.create-check.outputs.check-id && always() with: token: $\{{ secrets.GITHUB_TOKEN }} conclusion: $\{{ job.status }} diff --git a/smoke-tests/test/fixtures/setup.js b/smoke-tests/test/fixtures/setup.js index e4f7dece22d6e..e5302104f2503 100644 --- a/smoke-tests/test/fixtures/setup.js +++ b/smoke-tests/test/fixtures/setup.js @@ -7,7 +7,7 @@ const MockRegistry = require('@npmcli/mock-registry') const http = require('http') const { createProxy } = require('proxy') -const { SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI, PATH, Path } = process.env +const { SMOKE_PUBLISH_TARBALL, CI, PATH, Path } = process.env const DEFAULT_REGISTRY = new URL('https://registry.npmjs.org/') const MOCK_REGISTRY = new URL('http://smoke-test-registry.club/') @@ -75,7 +75,7 @@ const getCleanPaths = async () => { module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy = false } = {}) => { const debugLog = debug || CI ? (...a) => t.comment(...a) : () => {} - debugLog({ SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI }) + debugLog({ SMOKE_PUBLISH_TARBALL, CI }) const cleanPaths = await getCleanPaths() @@ -219,7 +219,7 @@ module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy const npmLocal = async (...args) => { const [{ force = false }] = getOpts(...args) - if (SMOKE_PUBLISH_NPM && !force) { + if (SMOKE_PUBLISH_TARBALL && !force) { throw new Error('npmLocal cannot be called during smoke-publish') } return baseNpm({ @@ -251,7 +251,7 @@ module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy return { npmPath, npmLocal, - npm: SMOKE_PUBLISH_NPM ? npmPath : npm, + npm: SMOKE_PUBLISH_TARBALL ? npmPath : npm, spawn: baseSpawn, readFile, getPath, @@ -269,6 +269,6 @@ 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 = !!SMOKE_PUBLISH_TARBALL module.exports.SMOKE_PUBLISH_TARBALL = SMOKE_PUBLISH_TARBALL module.exports.MOCK_REGISTRY = MOCK_REGISTRY diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index c316369d296e7..b7177a0fec816 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -120,12 +120,11 @@ t.test('publish and replace global self', async t => { }) } - const npmInstall = async (useNpm, opts) => { + const npmInstall = async (useNpm) => { await npmPackage({ manifest: { packuments: [publishedPackument] }, tarballs: { [version]: tarball }, times: 3, - ...opts, }) await fs.rm(cache, { recursive: true, force: true }) await useNpm('install', 'npm@latest', '--global')