Skip to content

Commit 9ffeb69

Browse files
authored
chore: use proxy instead of http-proxy (#6814)
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.
1 parent 2207628 commit 9ffeb69

File tree

7 files changed

+29
-87
lines changed

7 files changed

+29
-87
lines changed

DEPENDENCIES.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,6 @@ graph LR;
717717
npmcli-run-script-->npmcli-promise-spawn["@npmcli/promise-spawn"];
718718
npmcli-run-script-->read-package-json-fast;
719719
npmcli-run-script-->which;
720-
npmcli-smoke-tests-->http-proxy;
721720
npmcli-smoke-tests-->npmcli-eslint-config["@npmcli/eslint-config"];
722721
npmcli-smoke-tests-->npmcli-mock-registry["@npmcli/mock-registry"];
723722
npmcli-smoke-tests-->npmcli-promise-spawn["@npmcli/promise-spawn"];

package-lock.json

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6805,26 +6805,6 @@
68056805
"dev": true,
68066806
"peer": true
68076807
},
6808-
"node_modules/follow-redirects": {
6809-
"version": "1.15.2",
6810-
"resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz",
6811-
"integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==",
6812-
"dev": true,
6813-
"funding": [
6814-
{
6815-
"type": "individual",
6816-
"url": "https://github.com/sponsors/RubenVerborgh"
6817-
}
6818-
],
6819-
"engines": {
6820-
"node": ">=4.0"
6821-
},
6822-
"peerDependenciesMeta": {
6823-
"debug": {
6824-
"optional": true
6825-
}
6826-
}
6827-
},
68286808
"node_modules/for-each": {
68296809
"version": "0.3.3",
68306810
"resolved": "https://registry.npmjs.org/for-each/-/for-each-0.3.3.tgz",
@@ -7618,20 +7598,6 @@
76187598
"integrity": "sha512-er295DKPVsV82j5kw1Gjt+ADA/XYHsajl82cGNQG2eyoPkvgUhX+nDIyelzhIWbbsXP39EHcI6l5tYs2FYqYXQ==",
76197599
"inBundle": true
76207600
},
7621-
"node_modules/http-proxy": {
7622-
"version": "1.18.1",
7623-
"resolved": "https://registry.npmjs.org/http-proxy/-/http-proxy-1.18.1.tgz",
7624-
"integrity": "sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==",
7625-
"dev": true,
7626-
"dependencies": {
7627-
"eventemitter3": "^4.0.0",
7628-
"follow-redirects": "^1.0.0",
7629-
"requires-port": "^1.0.0"
7630-
},
7631-
"engines": {
7632-
"node": ">=8.0.0"
7633-
}
7634-
},
76357601
"node_modules/http-proxy-agent": {
76367602
"version": "5.0.0",
76377603
"resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-5.0.0.tgz",
@@ -17377,7 +17343,6 @@
1737717343
"@npmcli/mock-registry": "^1.0.0",
1737817344
"@npmcli/promise-spawn": "^7.0.0",
1737917345
"@npmcli/template-oss": "4.19.0",
17380-
"http-proxy": "^1.18.1",
1738117346
"proxy": "^2.1.1",
1738217347
"semver": "^7.5.4",
1738317348
"tap": "^16.3.8",

smoke-tests/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
"@npmcli/mock-registry": "^1.0.0",
2323
"@npmcli/promise-spawn": "^7.0.0",
2424
"@npmcli/template-oss": "4.19.0",
25-
"http-proxy": "^1.18.1",
2625
"proxy": "^2.1.1",
2726
"semver": "^7.5.4",
2827
"tap": "^16.3.8",
@@ -38,6 +37,7 @@
3837
"tap": {
3938
"no-coverage": true,
4039
"timeout": 600,
40+
"jobs": 1,
4141
"test-ignore": "fixtures/*",
4242
"nyc-arg": [
4343
"--exclude",

smoke-tests/test/fixtures/setup.js

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ const which = require('which')
55
const spawn = require('@npmcli/promise-spawn')
66
const MockRegistry = require('@npmcli/mock-registry')
77
const http = require('http')
8-
const httpProxy = require('http-proxy')
8+
const { createProxy } = require('proxy')
9+
10+
const { SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI, PATH, Path } = process.env
911

10-
const { SMOKE_PUBLISH_NPM, SMOKE_PUBLISH_TARBALL, CI, PATH, Path, TAP_CHILD_ID = '0' } = process.env
11-
const PROXY_PORT = 12345 + (+TAP_CHILD_ID)
12-
const HTTP_PROXY = `http://localhost:${PROXY_PORT}/`
1312
const DEFAULT_REGISTRY = new URL('https://registry.npmjs.org/')
13+
const MOCK_REGISTRY = new URL('http://smoke-test-registry.club/')
1414

1515
const NODE_PATH = process.execPath
1616
const CLI_ROOT = resolve(process.cwd(), '..')
@@ -73,25 +73,7 @@ const getCleanPaths = async () => {
7373
})
7474
}
7575

76-
const createRegistry = async (t, { debug, ...opts } = {}) => {
77-
const registry = new MockRegistry({
78-
tap: t,
79-
registry: 'http://smoke-test-registry.club/',
80-
debug,
81-
strict: true,
82-
...opts,
83-
})
84-
85-
const proxy = httpProxy.createProxyServer({})
86-
const server = http.createServer((req, res) => proxy.web(req, res, { target: registry.origin }))
87-
await new Promise(res => server.listen(PROXY_PORT, res))
88-
89-
t.teardown(() => server.close())
90-
91-
return registry
92-
}
93-
94-
module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = {}) => {
76+
module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy = false } = {}) => {
9577
const debugLog = debug || CI ? (...a) => console.error(...a) : () => {}
9678
const cleanPaths = await getCleanPaths()
9779

@@ -115,11 +97,21 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = {
11597
globalNodeModules: join(root, 'global', GLOBAL_NODE_MODULES),
11698
}
11799

118-
const liveRegistry = _registry === false
119-
const USE_PROXY = !liveRegistry
120-
const registry = liveRegistry
121-
? DEFAULT_REGISTRY
122-
: await createRegistry(t, { ..._registry, debug })
100+
const registry = !mockRegistry ? DEFAULT_REGISTRY : new MockRegistry({
101+
tap: t,
102+
registry: MOCK_REGISTRY,
103+
debug,
104+
strict: true,
105+
})
106+
107+
const proxyEnv = {}
108+
if (useProxy || mockRegistry) {
109+
useProxy = true
110+
const proxyServer = createProxy(http.createServer())
111+
await new Promise(res => proxyServer.listen(0, res))
112+
t.teardown(() => proxyServer.close())
113+
proxyEnv.HTTP_PROXY = new URL(`http://localhost:${proxyServer.address().port}`)
114+
}
123115

124116
// update notifier should never be written
125117
t.afterEach((t) => {
@@ -143,7 +135,6 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = {
143135
}
144136
return s
145137
.split(relative(CLI_ROOT, t.testdirName)).join('{TESTDIR}')
146-
.split(HTTP_PROXY).join('{PROXY_REGISTRY}')
147138
.split(registry.origin).join('{REGISTRY}')
148139
.replace(/\\+/g, '/')
149140
.replace(/\r\n/g, '\n')
@@ -188,12 +179,12 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = {
188179
}
189180

190181
const baseNpm = async (...a) => {
191-
const [{ cwd, cmd, argv = [], proxy = USE_PROXY, ...opts }, args] = getOpts(...a)
182+
const [{ cwd, cmd, argv = [], proxy = useProxy, ...opts }, args] = getOpts(...a)
192183

193184
const isGlobal = args.some(arg => ['-g', '--global', '--global=true'].includes(arg))
194185

195186
const defaultFlags = [
196-
proxy ? `--registry=${HTTP_PROXY}` : null,
187+
`--registry=${registry.origin}`,
197188
`--cache=${paths.cache}`,
198189
`--prefix=${isGlobal ? paths.global : cwd}`,
199190
`--userconfig=${paths.userConfig}`,
@@ -217,7 +208,7 @@ module.exports = async (t, { testdir = {}, debug, registry: _registry = {} } = {
217208
cwd,
218209
env: {
219210
...opts.env,
220-
...proxy ? { HTTP_PROXY } : {},
211+
...proxy ? proxyEnv : {},
221212
},
222213
...opts,
223214
})
@@ -274,5 +265,4 @@ module.exports.CLI_ROOT = CLI_ROOT
274265
module.exports.WINDOWS = WINDOWS
275266
module.exports.SMOKE_PUBLISH = !!SMOKE_PUBLISH_NPM
276267
module.exports.SMOKE_PUBLISH_TARBALL = SMOKE_PUBLISH_TARBALL
277-
module.exports.HTTP_PROXY = HTTP_PROXY
278-
module.exports.PROXY_PORT = PROXY_PORT
268+
module.exports.MOCK_REGISTRY = MOCK_REGISTRY

smoke-tests/test/max-listeners.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const getFixture = (p) => require(
1010

1111
const runTest = async (t, { env } = {}) => {
1212
const { npm } = await setup(t, {
13-
registry: false,
13+
mockRegistry: false,
1414
testdir: {
1515
project: {
1616
'package.json': getFixture('package.json'),

smoke-tests/test/npm-replace-global.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ t.test('publish and replace global self', async t => {
110110
} = await setupNpmGlobal(t, {
111111
testdir: {
112112
home: {
113-
'.npmrc': `${setup.HTTP_PROXY.slice(5)}:_authToken = test-token`,
113+
'.npmrc': `//${setup.MOCK_REGISTRY.host}/:_authToken = test-token`,
114114
},
115115
},
116116
})

smoke-tests/test/proxy.js

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,12 @@
11
const t = require('tap')
22
const setup = require('./fixtures/setup.js')
3-
const { createProxy } = require('proxy')
4-
const http = require('http')
53

64
t.test('proxy', async t => {
7-
const { PROXY_PORT: PORT } = setup
8-
95
const { npm, readFile } = await setup(t, {
10-
registry: false,
11-
testdir: {
12-
home: {
13-
'.npmrc': `proxy = "http://localhost:${PORT}/"`,
14-
},
15-
},
6+
mockRegistry: false,
7+
useProxy: true,
168
})
179

18-
const server = createProxy(http.createServer())
19-
await new Promise(res => server.listen(PORT, res))
20-
t.teardown(() => server.close())
21-
2210
await npm('install', '[email protected]')
2311

2412
t.strictSame(await readFile('package.json'), {

0 commit comments

Comments
 (0)