From 11f314d358f0db3b1a0b318190412a7ebf7de3ba Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 15 Sep 2023 16:16:33 -0700 Subject: [PATCH] chore: use proxy instead of http-proxy The proxy package is more spec compliant and works out-of-the-box so we no longer have to pass --registry=PROXY to the tests. The goal here is to make it simpler to help debug #6793. --- DEPENDENCIES.md | 1 - package-lock.json | 35 ---------------- smoke-tests/package.json | 2 +- smoke-tests/test/fixtures/setup.js | 58 +++++++++++--------------- smoke-tests/test/max-listeners.js | 2 +- smoke-tests/test/npm-replace-global.js | 2 +- smoke-tests/test/proxy.js | 16 +------ 7 files changed, 29 insertions(+), 87 deletions(-) diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index 0adcea2c7f204..d260d54a99bf6 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -716,7 +716,6 @@ graph LR; npmcli-run-script-->npmcli-promise-spawn["@npmcli/promise-spawn"]; npmcli-run-script-->read-package-json-fast; npmcli-run-script-->which; - npmcli-smoke-tests-->http-proxy; 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 93f52fef38f70..860fdadecd415 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6803,26 +6803,6 @@ "dev": true, "peer": true }, - "node_modules/follow-redirects": { - "version": "1.15.2", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz", - "integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==", - "dev": true, - "funding": [ - { - "type": "individual", - "url": "https://github.com/sponsors/RubenVerborgh" - } - ], - "engines": { - "node": ">=4.0" - }, - "peerDependenciesMeta": { - "debug": { - "optional": true - } - } - }, "node_modules/for-each": { "version": "0.3.3", "resolved": "https://registry.npmjs.org/for-each/-/for-each-0.3.3.tgz", @@ -7616,20 +7596,6 @@ "integrity": "sha512-er295DKPVsV82j5kw1Gjt+ADA/XYHsajl82cGNQG2eyoPkvgUhX+nDIyelzhIWbbsXP39EHcI6l5tYs2FYqYXQ==", "inBundle": true }, - "node_modules/http-proxy": { - "version": "1.18.1", - "resolved": "https://registry.npmjs.org/http-proxy/-/http-proxy-1.18.1.tgz", - "integrity": "sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==", - "dev": true, - "dependencies": { - "eventemitter3": "^4.0.0", - "follow-redirects": "^1.0.0", - "requires-port": "^1.0.0" - }, - "engines": { - "node": ">=8.0.0" - } - }, "node_modules/http-proxy-agent": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-5.0.0.tgz", @@ -17375,7 +17341,6 @@ "@npmcli/mock-registry": "^1.0.0", "@npmcli/promise-spawn": "^7.0.0", "@npmcli/template-oss": "4.19.0", - "http-proxy": "^1.18.1", "proxy": "^2.1.1", "semver": "^7.5.4", "tap": "^16.3.8", diff --git a/smoke-tests/package.json b/smoke-tests/package.json index 805f7ae7a7587..665cb2215838d 100644 --- a/smoke-tests/package.json +++ b/smoke-tests/package.json @@ -22,7 +22,6 @@ "@npmcli/mock-registry": "^1.0.0", "@npmcli/promise-spawn": "^7.0.0", "@npmcli/template-oss": "4.19.0", - "http-proxy": "^1.18.1", "proxy": "^2.1.1", "semver": "^7.5.4", "tap": "^16.3.8", @@ -38,6 +37,7 @@ "tap": { "no-coverage": true, "timeout": 600, + "jobs": 1, "test-ignore": "fixtures/*", "nyc-arg": [ "--exclude", diff --git a/smoke-tests/test/fixtures/setup.js b/smoke-tests/test/fixtures/setup.js index fc2ce9cacedfc..e6c5de61b23fc 100644 --- a/smoke-tests/test/fixtures/setup.js +++ b/smoke-tests/test/fixtures/setup.js @@ -5,12 +5,12 @@ const which = require('which') const spawn = require('@npmcli/promise-spawn') const MockRegistry = require('@npmcli/mock-registry') const http = require('http') -const httpProxy = require('http-proxy') +const { createProxy } = require('proxy') + +const { SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI, PATH, Path } = process.env -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 DEFAULT_REGISTRY = new URL('https://registry.npmjs.org/') +const MOCK_REGISTRY = new URL('http://smoke-test-registry.club/') const NODE_PATH = process.execPath const CLI_ROOT = resolve(process.cwd(), '..') @@ -73,25 +73,7 @@ const getCleanPaths = async () => { }) } -const createRegistry = async (t, { debug, ...opts } = {}) => { - const registry = new MockRegistry({ - tap: t, - registry: 'http://smoke-test-registry.club/', - debug, - strict: true, - ...opts, - }) - - 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, registry: _registry = {} } = {}) => { +module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy = false } = {}) => { const debugLog = debug || CI ? (...a) => console.error(...a) : () => {} const cleanPaths = await getCleanPaths() @@ -115,11 +97,21 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = { globalNodeModules: join(root, 'global', GLOBAL_NODE_MODULES), } - const liveRegistry = _registry === false - const USE_PROXY = !liveRegistry - const registry = liveRegistry - ? DEFAULT_REGISTRY - : await createRegistry(t, { ..._registry, debug }) + const registry = !mockRegistry ? DEFAULT_REGISTRY : new MockRegistry({ + tap: t, + registry: MOCK_REGISTRY, + debug, + strict: true, + }) + + const proxyEnv = {} + if (useProxy || mockRegistry) { + useProxy = true + const proxyServer = createProxy(http.createServer()) + await new Promise(res => proxyServer.listen(0, res)) + t.teardown(() => proxyServer.close()) + proxyEnv.HTTP_PROXY = new URL(`http://localhost:${proxyServer.address().port}`) + } // update notifier should never be written t.afterEach((t) => { @@ -143,7 +135,6 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = { } return s .split(relative(CLI_ROOT, t.testdirName)).join('{TESTDIR}') - .split(HTTP_PROXY).join('{PROXY_REGISTRY}') .split(registry.origin).join('{REGISTRY}') .replace(/\\+/g, '/') .replace(/\r\n/g, '\n') @@ -188,12 +179,12 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = { } const baseNpm = async (...a) => { - const [{ cwd, cmd, argv = [], proxy = USE_PROXY, ...opts }, args] = getOpts(...a) + const [{ cwd, cmd, argv = [], proxy = useProxy, ...opts }, args] = getOpts(...a) const isGlobal = args.some(arg => ['-g', '--global', '--global=true'].includes(arg)) const defaultFlags = [ - proxy ? `--registry=${HTTP_PROXY}` : null, + `--registry=${registry.origin}`, `--cache=${paths.cache}`, `--prefix=${isGlobal ? paths.global : cwd}`, `--userconfig=${paths.userConfig}`, @@ -217,7 +208,7 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = { cwd, env: { ...opts.env, - ...proxy ? { HTTP_PROXY } : {}, + ...proxy ? proxyEnv : {}, }, ...opts, }) @@ -274,5 +265,4 @@ 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 -module.exports.HTTP_PROXY = HTTP_PROXY -module.exports.PROXY_PORT = PROXY_PORT +module.exports.MOCK_REGISTRY = MOCK_REGISTRY diff --git a/smoke-tests/test/max-listeners.js b/smoke-tests/test/max-listeners.js index 6b9e242cc0746..ac273c89334f2 100644 --- a/smoke-tests/test/max-listeners.js +++ b/smoke-tests/test/max-listeners.js @@ -10,7 +10,7 @@ const getFixture = (p) => require( const runTest = async (t, { env } = {}) => { const { npm } = await setup(t, { - registry: false, + mockRegistry: false, testdir: { project: { 'package.json': getFixture('package.json'), diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 4e6b78b881312..432935d49fd38 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -110,7 +110,7 @@ t.test('publish and replace global self', async t => { } = await setupNpmGlobal(t, { testdir: { home: { - '.npmrc': `${setup.HTTP_PROXY.slice(5)}:_authToken = test-token`, + '.npmrc': `//${setup.MOCK_REGISTRY.host}/:_authToken = test-token`, }, }, }) diff --git a/smoke-tests/test/proxy.js b/smoke-tests/test/proxy.js index f437572de79e0..8746c546de55f 100644 --- a/smoke-tests/test/proxy.js +++ b/smoke-tests/test/proxy.js @@ -1,24 +1,12 @@ const t = require('tap') const setup = require('./fixtures/setup.js') -const { createProxy } = require('proxy') -const http = require('http') t.test('proxy', async t => { - const { PROXY_PORT: PORT } = setup - const { npm, readFile } = await setup(t, { - registry: false, - testdir: { - home: { - '.npmrc': `proxy = "http://localhost:${PORT}/"`, - }, - }, + mockRegistry: false, + useProxy: true, }) - const server = createProxy(http.createServer()) - await new Promise(res => server.listen(PORT, res)) - t.teardown(() => server.close()) - await npm('install', 'abbrev@1.0.4') t.strictSame(await readFile('package.json'), {