1
0
Эх сурвалжийг харах

test: start every suite from an environment with no proxy

A developer's Clash and a CI runner's squid both export HTTP_PROXY and its
siblings. Now that the harness honors them, an ambient value decides test
outcomes: this PR has already recorded a proxy's 502 page as a snapshot's
expected output, and let a runner's own export stand in for "what the user
exported" in an assertion about inherited names.

A Vitest setup file clears the proxy names before any suite runs, wired into
every configuration that declares a setup. Real-API e2e is cleared too: before
proxy support existed every request connected directly and that suite passed, so
direct is the environment it is known to work in.

`NODE_USE_ENV_PROXY` cannot be cleared this way — Node samples the proxy
environment at process start — and the module says so. A proxy application never
exports it, and the eight names one does export are fully handled: with all of
them set, the affected suites pass.

The wiring is what regresses, so that is what the test pins. Configurations are
discovered rather than listed, because the web suites carry no setup today and a
hand-written list would let one of them gain a setup without gaining this one.

`plugin.spec.ts` kept its own copy of the eight names to guard against the
machine; it never sets a proxy variable itself, so the setup replaces that
entirely. `install.spec.ts` had one assertion waiting on a DNS miss with no
deadline, which timed out once under load.
Yichen Jiang 2 долоо хоног өмнө
parent
commit
03d2c54db1

+ 4 - 1
packages/net/http-proxy/tests/install.spec.ts

@@ -142,7 +142,10 @@ describe('installGlobalProxy', () => {
     // reuses the HTTP one, tunnelling the scheme the diagnostic told the user stayed direct.
     const dispose = await installGlobalProxy({ httpProxy: proxyUrl, noProxy: '', source: 'env' })
     try {
-      await expect(fetch('https://refused-scheme.invalid/')).rejects.toThrow()
+      // The direct path here fails on a DNS miss whose latency is the machine's resolver to decide;
+      // the deadline bounds it. Either rejection proves the same thing — no CONNECT reached the
+      // proxy — and a proxied hop would have answered in milliseconds instead.
+      await expect(fetch('https://refused-scheme.invalid/', { signal: AbortSignal.timeout(1500) })).rejects.toThrow()
       expect(proxied).toEqual([])
       // The same policy still tunnels http, so the empty expectation above is not vacuous.
       await expect((await fetch(originUrl)).text()).resolves.toBe('VIA-PROXY')

+ 5 - 21
packages/net/http-proxy/tests/plugin.spec.ts

@@ -1,33 +1,17 @@
-import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it } from 'vitest'
 import { Context } from '@deepseek-ai/cordis'
 import InvariantRegistry from '@deepseek-ai/dsh-invariants'
 import { getGlobalDispatcher } from 'undici'
+import { PROXY_ENV_NAMES } from '../src/policy.ts'
 import * as HttpProxy from '../src/index.ts'
 import * as HttpProxyInvariant from '../src/invariant.ts'
 
 const PROXY = 'http://127.0.0.1:7897'
 
-/**
- * Every proxy name in both casings. The suite clears all of them so a developer's own exported proxy
- * cannot decide the outcome — the lowercase names matter most, since resolution reads those first.
- */
-const PROXY_ENV_NAMES = [
-  'http_proxy', 'HTTP_PROXY', 'https_proxy', 'HTTPS_PROXY',
-  'no_proxy', 'NO_PROXY', 'all_proxy', 'ALL_PROXY',
-] as const
-
-let saved: Record<string, string | undefined> = {}
-
-beforeEach(() => {
-  saved = Object.fromEntries(PROXY_ENV_NAMES.map(name => [name, process.env[name]]))
-  for (const name of PROXY_ENV_NAMES) Reflect.deleteProperty(process.env, name)
-})
-
+// `scripts/test-proxy-environment.ts` clears the machine's proxy variables before any suite runs,
+// so each test starts from nothing and only has to undo what `withEnv` set.
 afterEach(() => {
-  for (const [name, value] of Object.entries(saved)) {
-    if (value === undefined) Reflect.deleteProperty(process.env, name)
-    else process.env[name] = value
-  }
+  for (const name of PROXY_ENV_NAMES) Reflect.deleteProperty(process.env, name)
 })
 
 /** The launcher normally provides a snapshot; without one the plugin reads the process environment. */

+ 43 - 0
scripts/test-proxy-environment.spec.ts

@@ -0,0 +1,43 @@
+import { readFileSync } from 'node:fs'
+import { describe, expect, it } from 'vitest'
+import { PROXY_ENV_NAMES } from '../packages/net/http-proxy/src/policy.ts'
+import { clearAmbientProxyEnv, TEST_PROXY_SETUP_FILE, vitestConfigFiles } from './test-proxy-environment.ts'
+
+describe('ambient proxy environment', () => {
+  it('clears every name the policy resolver reads, in both casings', () => {
+    const env: NodeJS.ProcessEnv = {
+      HTTP_PROXY: 'http://p:1', http_proxy: 'http://p:1',
+      HTTPS_PROXY: 'http://p:1', https_proxy: 'http://p:1',
+      ALL_PROXY: 'http://p:1', all_proxy: 'http://p:1',
+      NO_PROXY: 'example.com', no_proxy: 'example.com',
+      NODE_USE_ENV_PROXY: '1',
+      PATH: '/usr/bin',
+    }
+    expect(clearAmbientProxyEnv(env)).toHaveLength(PROXY_ENV_NAMES.length + 1)
+    expect(env).toEqual({ PATH: '/usr/bin' })
+  })
+
+  it('reports only the names that were set, and touches nothing else', () => {
+    const env: NodeJS.ProcessEnv = { all_proxy: 'http://p:1', HOME: '/home/me' }
+    expect(clearAmbientProxyEnv(env)).toEqual(['all_proxy'])
+    expect(env).toEqual({ HOME: '/home/me' })
+  })
+
+  // A runtime assertion that this process is clear would pass either way: importing the module
+  // above already ran it. What can actually regress is the wiring — a new Vitest project, or a
+  // config that lists only the invariant host — so that is what this pins.
+  const declared = vitestConfigFiles()
+    .map(config => ({ config, slots: readFileSync(config, 'utf8').match(/setupFiles: \[[^\]]*\]/g) ?? [] }))
+    .filter(entry => entry.slots.length > 0)
+
+  it('finds the configurations that declare a setup at all', () => {
+    // Guards the discovery itself: a glob that stopped matching would make every case below vacuous.
+    expect(declared.map(entry => entry.config)).toEqual([
+      'vitest.config.ts', 'vitest.e2e.config.ts', 'vitest.expected.config.ts', 'vitest.snapshot.config.ts',
+    ])
+  })
+
+  it.each(declared)('$config runs the setup in every setupFiles it declares', ({ slots }) => {
+    for (const slot of slots) expect(slot).toContain(TEST_PROXY_SETUP_FILE)
+  })
+})

+ 63 - 0
scripts/test-proxy-environment.ts

@@ -0,0 +1,63 @@
+/**
+ * Remove the machine's proxy configuration from every Vitest process.
+ *
+ * A developer's Clash and a CI runner's squid both export `HTTP_PROXY` and its siblings. Now that
+ * the harness honors them, an ambient value silently decides test outcomes: a request meant for a
+ * local fixture server is sent to a proxy that cannot resolve the fixture's hostname, and the
+ * proxy's error page is recorded as the expected output. The same value also stands in for "what
+ * the user exported" in any assertion about inherited proxy names.
+ *
+ * Clearing here gives every suite one known starting environment, so a test that needs a proxy sets
+ * exactly the names it means to exercise. Suites that spawn a real `dsh` still clear the child's
+ * environment themselves — they must hold whether or not a Vitest setup ran.
+ *
+ * One name resists this: `NODE_USE_ENV_PROXY`. Node samples the proxy environment when the process
+ * starts, so deleting the variable from a setup file cannot unbind the built-in `fetch` it already
+ * configured. A shell that exports it must unset it before running the suite. The names a proxy
+ * application or a corporate profile actually exports — the eight below — are fully handled, because
+ * only this repository's own resolver reads them and it runs after this.
+ *
+ * Real-API e2e is cleared too. Before proxy support existed every request connected directly and
+ * that suite passed, so a direct connection is the environment it is known to work in; leaving the
+ * ambient proxy in place would newly stake it on the proxy reaching the provider.
+ * @module
+ */
+
+import { globSync } from 'node:fs'
+import { resolve } from 'node:path'
+import { PROXY_ENV_NAMES } from '../packages/net/http-proxy/src/policy.ts'
+
+/** The flag a Node process reads before honoring the names above; ambient in the same way. */
+const NODE_PROXY_FLAG = 'NODE_USE_ENV_PROXY'
+
+/** This module's path as a `setupFiles` entry, so its own wiring test names it once. */
+export const TEST_PROXY_SETUP_FILE = './scripts/test-proxy-environment.ts'
+
+/**
+ * Every Vitest configuration in the repository, discovered rather than listed: the web suites carry
+ * no `setupFiles` today, and a hand-written list would let one of them gain a setup without gaining
+ * this one. The wiring test asserts only over the configurations that declare a setup at all.
+ *
+ * @returns repository-relative config paths, sorted.
+ */
+export function vitestConfigFiles(): string[] {
+  return globSync('vitest*.ts', { cwd: resolve(import.meta.dirname, '..') }).sort()
+}
+
+/**
+ * Delete every proxy name from one environment.
+ *
+ * @param env - the environment to clear.
+ * @returns the names that carried a value, in the order checked.
+ */
+export function clearAmbientProxyEnv(env: NodeJS.ProcessEnv): string[] {
+  const cleared: string[] = []
+  for (const name of [...PROXY_ENV_NAMES, NODE_PROXY_FLAG]) {
+    if (env[name] === undefined) continue
+    cleared.push(name)
+    Reflect.deleteProperty(env, name)
+  }
+  return cleared
+}
+
+clearAmbientProxyEnv(process.env)

+ 3 - 3
vitest.config.ts

@@ -149,7 +149,7 @@ const processBoundTests = [
 export default defineConfig({
   plugins: [pathsPlugin(), standardDecoratorPlugin()],
   test: {
-    setupFiles: ['./scripts/test-invariants.ts'],
+    setupFiles: ['./scripts/test-proxy-environment.ts', './scripts/test-invariants.ts'],
     // .tsx: client component specs (jsdom via per-file @vitest-environment pragma).
     include: testIncludes,
     exclude: platformUnsupportedTests,
@@ -165,7 +165,7 @@ export default defineConfig({
           // MaybeLocal in cjs_lexer::Parse) from worker threads on macOS,
           // Linux, and Windows. Forked workers avoid that shared thread path.
           pool: 'forks',
-          setupFiles: ['./scripts/test-invariants.ts'],
+          setupFiles: ['./scripts/test-proxy-environment.ts', './scripts/test-invariants.ts'],
           include: testIncludes,
           exclude: [
             ...platformUnsupportedTests,
@@ -180,7 +180,7 @@ export default defineConfig({
           name: 'process-bound',
           execArgv: vitestExecArgv,
           pool: 'forks',
-          setupFiles: ['./scripts/test-invariants.ts'],
+          setupFiles: ['./scripts/test-proxy-environment.ts', './scripts/test-invariants.ts'],
           include: processBoundTests,
           exclude: [
             ...platformUnsupportedTests,

+ 1 - 1
vitest.e2e.config.ts

@@ -39,7 +39,7 @@ export default defineConfig({
   plugins: [tsconfigPaths({ projects: ['./tsconfig.base.json'] }), standardDecoratorPlugin()],
   test: {
     execArgv: vitestExecArgv,
-    setupFiles: ['./scripts/test-invariants.ts'],
+    setupFiles: ['./scripts/test-proxy-environment.ts', './scripts/test-invariants.ts'],
     // apps/cli only, not apps/*: apps/web/tests/*.e2e.ts needs the built
     // frontend dist and runs under vitest.web.config.ts (the test:web job).
     include: ['packages/*/*/tests/**/*.e2e.ts', 'apps/cli/tests/**/*.e2e.ts'],

+ 1 - 1
vitest.expected.config.ts

@@ -8,7 +8,7 @@ export default defineConfig({
   plugins: [tsconfigPaths({ projects: ['./tsconfig.base.json'] }), standardDecoratorPlugin()],
   test: {
     execArgv: vitestExecArgv,
-    setupFiles: ['./scripts/test-invariants.ts'],
+    setupFiles: ['./scripts/test-proxy-environment.ts', './scripts/test-invariants.ts'],
     include: [
       'apps/cli/tests/**/*.expected.e2e.ts',
     ],

+ 1 - 1
vitest.snapshot.config.ts

@@ -43,7 +43,7 @@ export default defineConfig({
   plugins: [tsconfigPaths({ projects: ['./tsconfig.base.json'] }), standardDecoratorPlugin()],
   test: {
     execArgv: vitestExecArgv,
-    setupFiles: ['./scripts/test-invariants.ts'],
+    setupFiles: ['./scripts/test-proxy-environment.ts', './scripts/test-invariants.ts'],
     include: [
       'scripts/session-snapshot-corpus.corpus.ts',
       // The assembled Web snapshot executes generated client bundles; source