From 16671a5f049804dec2d420e9781d367f553ba601 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Sun, 2 Aug 2026 21:08:11 -0700 Subject: [PATCH 1/2] Fix: scrub ADK environment variables from unit test runs The three unit projects run in a process that inherits the developer's shell, and a lot of ADK production code reads configuration straight from process.env. A contributor with GOOGLE_CLOUD_PROJECT, DATABASE_URL or an OpenTelemetry endpoint exported runs a different suite from the one CI runs, because Actions runners export none of these: OTEL_EXPORTER_OTLP_ENDPOINT=http://127.0.0.1:4318 \ npx vitest run --project unit:core core/test/telemetry/setup_test.ts # 4 failed / 2 passed -- maybeSetOtelProviders() installs providers # from the ambient endpoint DATABASE_URL=bogus://nope \ npx vitest run --project unit:dev dev/test/cli/cli_test.ts # 9 failed / 12 passed -- "Unsupported session service URI" Wire tests/unit_setup.ts into the three unit:* projects. It deletes the 13 variables that core/src, dev/src or integrations/src read directly, plus the ADK_ prefix family, then writes them back in an afterAll. The prefix rule cannot be a literal list: feature_registry.ts composes ADK_ENABLE_${featureName} from an enum that grows. The scrub runs at module scope, not in a beforeEach. It has to land before the test file's module body, and a top-level beforeEach would run after apigee_llm_test.ts's beforeAll and delete the three variables that suite sets for itself. GOOGLE_APPLICATION_CREDENTIALS is deliberately absent: no ADK source reads it, and deleting it would not make auth hermetic anyway, since Application Default Credentials also resolve via the gcloud well-known file and the metadata server. integration, e2e and cross-language keep the ambient environment, which is how a developer supplies real credentials to them. --- tests/unit_setup.ts | 64 +++++++++++++++++++++++++++++++++++++++++++++ vitest.config.ts | 11 ++++++++ 2 files changed, 75 insertions(+) create mode 100644 tests/unit_setup.ts diff --git a/tests/unit_setup.ts b/tests/unit_setup.ts new file mode 100644 index 000000000..cb70ee513 --- /dev/null +++ b/tests/unit_setup.ts @@ -0,0 +1,64 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {afterAll} from 'vitest'; + +/** + * Environment variables that ADK production code reads directly. Unit tests + * must not inherit them from the developer's shell: a unit test that needs a + * value sets it explicitly, so that the suite behaves identically on a + * contributor's machine and in CI, where none of these are set. + * + * Every name here has a reader under `core/src`, `dev/src` or + * `integrations/src`. Ambient credential variables that ADK never reads + * (`GOOGLE_APPLICATION_CREDENTIALS`, `CLOUDSDK_*`) are deliberately absent: + * deleting them would not make auth hermetic, because Application Default + * Credentials also resolve via the gcloud well-known file and the metadata + * server. + */ +export const SCRUBBED_ENV_VARS: readonly string[] = [ + 'APIGEE_PROXY_URL', + 'DATABASE_URL', + 'GEMINI_API_KEY', + 'GOOGLE_API_KEY', + 'GOOGLE_CLOUD_AGENT_ENGINE_ID', + 'GOOGLE_CLOUD_LOCATION', + 'GOOGLE_CLOUD_PROJECT', + 'GOOGLE_GENAI_API_KEY', + 'GOOGLE_GENAI_USE_VERTEXAI', + 'OTEL_EXPORTER_OTLP_ENDPOINT', + 'OTEL_EXPORTER_OTLP_LOGS_ENDPOINT', + 'OTEL_EXPORTER_OTLP_METRICS_ENDPOINT', + 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', +]; + +/** + * Families scrubbed by prefix. Feature flags are read under names built at + * runtime (`ADK_ENABLE_${featureName}` in feature_registry.ts), so the ADK + * family cannot be enumerated. + */ +export const SCRUBBED_ENV_PREFIXES: readonly string[] = ['ADK_']; + +function isScrubbed(name: string): boolean { + return ( + SCRUBBED_ENV_VARS.includes(name) || + SCRUBBED_ENV_PREFIXES.some((prefix) => name.startsWith(prefix)) + ); +} + +const inheritedEnv: Record = {}; + +for (const name of Object.keys(process.env)) { + const value = process.env[name]; + if (value !== undefined && isScrubbed(name)) { + inheritedEnv[name] = value; + delete process.env[name]; + } +} + +afterAll(() => { + Object.assign(process.env, inheritedEnv); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 1b7eaee63..f2483247b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -20,6 +20,14 @@ const INTEGRATION_HOOK_TIMEOUT_MS = 120000; */ const INTEGRATION_TEST_TIMEOUT_MS = 60000; +/** + * Setup module for the unit projects. It strips ADK-relevant environment + * variables so a developer's shell cannot change unit test behaviour. The + * `integration`, `e2e` and `cross-language` projects deliberately do not use + * it: they read real credentials. + */ +const UNIT_SETUP_FILE = path.resolve(__dirname, './tests/unit_setup.ts'); + export default defineConfig({ test: { poolOptions: { @@ -35,6 +43,7 @@ export default defineConfig({ test: { name: 'unit:core', environment: 'node', + setupFiles: [UNIT_SETUP_FILE], alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -49,6 +58,7 @@ export default defineConfig({ test: { name: 'unit:dev', environment: 'node', + setupFiles: [UNIT_SETUP_FILE], alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -63,6 +73,7 @@ export default defineConfig({ test: { name: 'unit:integrations', environment: 'node', + setupFiles: [UNIT_SETUP_FILE], alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( From 6c432e173249248f5ef6f88334d2a0a1d7d45e90 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Sun, 2 Aug 2026 21:08:11 -0700 Subject: [PATCH 2/2] Test: pin the env scrub policy and its per-project wiring One test file per unit project, because setupFiles is configured per Vitest project and each project's wiring needs its own proof. The duplication is deliberate; a shared helper would obscure what is being proven. The core file adds the two cases that pin the design rather than the config: that the scrub does not empty the environment, and that a value the test file sets for itself survives it. The second fails if anyone converts the module-scope scrub into a beforeEach. --- core/test/unit_setup_test.ts | 52 ++++++++++++++++++++++++++++ dev/test/unit_setup_test.ts | 29 ++++++++++++++++ integrations/test/unit_setup_test.ts | 29 ++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 core/test/unit_setup_test.ts create mode 100644 dev/test/unit_setup_test.ts create mode 100644 integrations/test/unit_setup_test.ts diff --git a/core/test/unit_setup_test.ts b/core/test/unit_setup_test.ts new file mode 100644 index 000000000..0f7d5dbbb --- /dev/null +++ b/core/test/unit_setup_test.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {afterAll, beforeAll, describe, expect, it} from 'vitest'; +import { + SCRUBBED_ENV_PREFIXES, + SCRUBBED_ENV_VARS, +} from '../../tests/unit_setup.js'; + +describe('unit:core test environment', () => { + it('removes every listed variable', () => { + for (const name of SCRUBBED_ENV_VARS) { + expect(process.env[name]).toBeUndefined(); + } + }); + + it('removes every variable in a scrubbed family', () => { + const remaining = Object.keys(process.env).filter((name) => + SCRUBBED_ENV_PREFIXES.some((prefix) => name.startsWith(prefix)), + ); + + expect(remaining).toEqual([]); + }); + + it('does not empty the environment', () => { + expect(Object.keys(process.env).length).toBeGreaterThan(0); + }); + + /** + * Pins the module-scope design: the scrub runs once, before the test file is + * imported. Converting it to a top-level `beforeEach` would delete this value + * between the `beforeAll` and the assertion -- and would break + * `core/test/models/apigee_llm_test.ts`, which sets three of the scrubbed + * variables in its own `beforeAll`. + */ + describe('a value set by the test file', () => { + beforeAll(() => { + process.env.GOOGLE_CLOUD_PROJECT = 'explicit-project'; + }); + + afterAll(() => { + delete process.env.GOOGLE_CLOUD_PROJECT; + }); + + it('survives the scrub', () => { + expect(process.env.GOOGLE_CLOUD_PROJECT).toBe('explicit-project'); + }); + }); +}); diff --git a/dev/test/unit_setup_test.ts b/dev/test/unit_setup_test.ts new file mode 100644 index 000000000..7fdaa1fac --- /dev/null +++ b/dev/test/unit_setup_test.ts @@ -0,0 +1,29 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {describe, expect, it} from 'vitest'; +import { + SCRUBBED_ENV_PREFIXES, + SCRUBBED_ENV_VARS, +} from '../../tests/unit_setup.js'; + +// `setupFiles` is configured per Vitest project, so each unit project needs its +// own test to prove its own wiring. +describe('unit:dev test environment', () => { + it('removes every listed variable', () => { + for (const name of SCRUBBED_ENV_VARS) { + expect(process.env[name]).toBeUndefined(); + } + }); + + it('removes every variable in a scrubbed family', () => { + const remaining = Object.keys(process.env).filter((name) => + SCRUBBED_ENV_PREFIXES.some((prefix) => name.startsWith(prefix)), + ); + + expect(remaining).toEqual([]); + }); +}); diff --git a/integrations/test/unit_setup_test.ts b/integrations/test/unit_setup_test.ts new file mode 100644 index 000000000..b2ea30af6 --- /dev/null +++ b/integrations/test/unit_setup_test.ts @@ -0,0 +1,29 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {describe, expect, it} from 'vitest'; +import { + SCRUBBED_ENV_PREFIXES, + SCRUBBED_ENV_VARS, +} from '../../tests/unit_setup.js'; + +// `setupFiles` is configured per Vitest project, so each unit project needs its +// own test to prove its own wiring. +describe('unit:integrations test environment', () => { + it('removes every listed variable', () => { + for (const name of SCRUBBED_ENV_VARS) { + expect(process.env[name]).toBeUndefined(); + } + }); + + it('removes every variable in a scrubbed family', () => { + const remaining = Object.keys(process.env).filter((name) => + SCRUBBED_ENV_PREFIXES.some((prefix) => name.startsWith(prefix)), + ); + + expect(remaining).toEqual([]); + }); +});