From 5ef9545436b6034941e715dfad20af9e8cb0900b Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Wed, 29 Jul 2026 19:43:37 -0700 Subject: [PATCH 1/3] test: unstub envs and globals between tests in every vitest project vi.stubEnv leaks into every later test unless unstubEnvs is enabled, so a stub set by one test silently changes the environment the next one reads. process.env is process-global, so the blast radius is every test file sharing the worker, not just the file that stubbed. Vitest projects inherit nothing from the root-level config, so both flags have to live in each project's own test block. The two env_stub_hermeticity_test.ts probes pin that placement: they are deliberately order-dependent and fail if the flags are set at the root or dropped from a project. --- core/test/utils/env_stub_hermeticity_test.ts | 34 ++++++++++++++++++++ dev/test/utils/env_stub_hermeticity_test.ts | 34 ++++++++++++++++++++ vitest.config.ts | 12 +++++++ 3 files changed, 80 insertions(+) create mode 100644 core/test/utils/env_stub_hermeticity_test.ts create mode 100644 dev/test/utils/env_stub_hermeticity_test.ts diff --git a/core/test/utils/env_stub_hermeticity_test.ts b/core/test/utils/env_stub_hermeticity_test.ts new file mode 100644 index 000000000..4ecb09265 --- /dev/null +++ b/core/test/utils/env_stub_hermeticity_test.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {describe, expect, it, vi} from 'vitest'; + +const PROBE_ENV_VAR = 'ADK_TEST_ENV_HERMETICITY_PROBE'; +const PROBE_GLOBAL = '__adkHermeticityProbe'; + +/** + * Pins `unstubEnvs` / `unstubGlobals` for the `unit:core` vitest project. + * + * Vitest projects inherit nothing from the root-level config, so the flags have + * to be set inside every project's own `test` block. These two tests are + * deliberately order-dependent: the first installs the stubs, the second proves + * the runner removed them before it started. Setting the flags at the root of + * `vitest.config.ts` instead of per project leaves the second test failing. + */ +describe('env and global stub hermeticity', () => { + it('applies stubs installed by this test', () => { + vi.stubEnv(PROBE_ENV_VAR, 'stubbed'); + vi.stubGlobal(PROBE_GLOBAL, 'stubbed'); + + expect(process.env[PROBE_ENV_VAR]).toBe('stubbed'); + expect(PROBE_GLOBAL in globalThis).toBe(true); + }); + + it('does not inherit stubs from the previous test', () => { + expect(process.env[PROBE_ENV_VAR]).toBeUndefined(); + expect(PROBE_GLOBAL in globalThis).toBe(false); + }); +}); diff --git a/dev/test/utils/env_stub_hermeticity_test.ts b/dev/test/utils/env_stub_hermeticity_test.ts new file mode 100644 index 000000000..89510ab42 --- /dev/null +++ b/dev/test/utils/env_stub_hermeticity_test.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {describe, expect, it, vi} from 'vitest'; + +const PROBE_ENV_VAR = 'ADK_TEST_ENV_HERMETICITY_PROBE'; +const PROBE_GLOBAL = '__adkHermeticityProbe'; + +/** + * Pins `unstubEnvs` / `unstubGlobals` for the `unit:dev` vitest project. + * + * Vitest projects inherit nothing from the root-level config, so the flags have + * to be set inside every project's own `test` block. These two tests are + * deliberately order-dependent: the first installs the stubs, the second proves + * the runner removed them before it started. Setting the flags at the root of + * `vitest.config.ts` instead of per project leaves the second test failing. + */ +describe('env and global stub hermeticity', () => { + it('applies stubs installed by this test', () => { + vi.stubEnv(PROBE_ENV_VAR, 'stubbed'); + vi.stubGlobal(PROBE_GLOBAL, 'stubbed'); + + expect(process.env[PROBE_ENV_VAR]).toBe('stubbed'); + expect(PROBE_GLOBAL in globalThis).toBe(true); + }); + + it('does not inherit stubs from the previous test', () => { + expect(process.env[PROBE_ENV_VAR]).toBeUndefined(); + expect(PROBE_GLOBAL in globalThis).toBe(false); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 0e27b0364..eb52ecf7f 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -22,6 +22,8 @@ export default defineConfig({ test: { name: 'unit:core', environment: 'node', + unstubEnvs: true, + unstubGlobals: true, alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -36,6 +38,8 @@ export default defineConfig({ test: { name: 'unit:dev', environment: 'node', + unstubEnvs: true, + unstubGlobals: true, alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -50,6 +54,8 @@ export default defineConfig({ test: { name: 'unit:integrations', environment: 'node', + unstubEnvs: true, + unstubGlobals: true, alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -64,6 +70,8 @@ export default defineConfig({ test: { name: 'integration', environment: 'node', + unstubEnvs: true, + unstubGlobals: true, alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -78,6 +86,8 @@ export default defineConfig({ test: { name: 'e2e', environment: 'node', + unstubEnvs: true, + unstubGlobals: true, alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( @@ -92,6 +102,8 @@ export default defineConfig({ test: { name: 'cross-language', environment: 'node', + unstubEnvs: true, + unstubGlobals: true, alias: { '@google/adk': path.resolve(__dirname, './core/src'), '@google/adk-integrations': path.resolve( From 82039b84fae3128f6a87d5805fb6ea0aa2a599b4 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Wed, 29 Jul 2026 19:43:48 -0700 Subject: [PATCH 2/3] test: stub env vars instead of mutating process.env in core tests vertex_ai_utils_test replaced process.env wholesale with a shallow copy, which swaps out Node's env object and cannot interoperate with vitest's automatic unstub. It now uses vi.stubEnv and neutralises the two variables it reads. setup_test and vertex_ai_session_service_test assert negative cases while leaving the ambient OTEL endpoint and express-mode variables in place, so they fail on a machine that exports them; both now stub those to undefined. --- .../vertex_ai_session_service_test.ts | 5 ++++ core/test/telemetry/setup_test.ts | 13 ++++++++- core/test/utils/vertex_ai_utils_test.ts | 28 +++++++++---------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/core/test/sessions/vertex_ai_session_service_test.ts b/core/test/sessions/vertex_ai_session_service_test.ts index f05b5c33c..ed88482db 100644 --- a/core/test/sessions/vertex_ai_session_service_test.ts +++ b/core/test/sessions/vertex_ai_session_service_test.ts @@ -77,6 +77,11 @@ describe('VertexAiSessionService', () => { let mockClient: MockSessions; beforeEach(() => { + // Express mode resolves a key from the ambient environment, which would + // stop the constructor throwing on a machine that exports these. + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', undefined); + vi.stubEnv('GOOGLE_API_KEY', undefined); + mockClient = { createInternal: vi.fn().mockResolvedValue({ name: 'operations/test-operation-id', diff --git a/core/test/telemetry/setup_test.ts b/core/test/telemetry/setup_test.ts index 3fa6aa0e8..d6afcb176 100644 --- a/core/test/telemetry/setup_test.ts +++ b/core/test/telemetry/setup_test.ts @@ -24,10 +24,21 @@ vi.mock('@opentelemetry/api'); vi.mock('@opentelemetry/api-logs'); vi.mock('../../src/utils/logger.js'); +const OTEL_ENDPOINT_ENV_VARS = [ + 'OTEL_EXPORTER_OTLP_ENDPOINT', + 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', + 'OTEL_EXPORTER_OTLP_METRICS_ENDPOINT', + 'OTEL_EXPORTER_OTLP_LOGS_ENDPOINT', +] as const; + describe('maybeSetOtelProviders', () => { beforeEach(() => { vi.clearAllMocks(); - vi.unstubAllEnvs(); + // The negative assertions below only hold if no endpoint variable is + // inherited from the ambient environment. + for (const name of OTEL_ENDPOINT_ENV_VARS) { + vi.stubEnv(name, undefined); + } }); afterEach(() => { diff --git a/core/test/utils/vertex_ai_utils_test.ts b/core/test/utils/vertex_ai_utils_test.ts index d397f143c..3525d949f 100644 --- a/core/test/utils/vertex_ai_utils_test.ts +++ b/core/test/utils/vertex_ai_utils_test.ts @@ -4,19 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import {beforeEach, describe, expect, it, vi} from 'vitest'; import {getExpressModeApiKey} from '../../src/utils/vertex_ai_utils.js'; describe('vertex_ai_utils', () => { describe('getExpressModeApiKey', () => { - const originalEnv = process.env; - + // Neutralise the ambient environment so the cases below read the same on a + // developer machine that exports these variables as they do on CI. Stubs + // are undone by `unstubEnvs` in vitest.config.ts, not by an afterEach. beforeEach(() => { - process.env = {...originalEnv}; - }); - - afterEach(() => { - process.env = originalEnv; + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', undefined); + vi.stubEnv('GOOGLE_API_KEY', undefined); }); it('should throw when both project and expressModeApiKey are provided', () => { @@ -38,33 +36,33 @@ describe('vertex_ai_utils', () => { }); it('should return undefined when GOOGLE_GENAI_USE_VERTEXAI is not set', () => { - delete process.env['GOOGLE_GENAI_USE_VERTEXAI']; + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', undefined); const result = getExpressModeApiKey(); expect(result).toBeUndefined(); }); it('should return undefined when GOOGLE_GENAI_USE_VERTEXAI is false', () => { - process.env['GOOGLE_GENAI_USE_VERTEXAI'] = 'false'; + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', 'false'); const result = getExpressModeApiKey(); expect(result).toBeUndefined(); }); it('should return expressModeApiKey when GOOGLE_GENAI_USE_VERTEXAI is true', () => { - process.env['GOOGLE_GENAI_USE_VERTEXAI'] = 'true'; + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', 'true'); const result = getExpressModeApiKey(undefined, undefined, 'my-api-key'); expect(result).toBe('my-api-key'); }); it('should return GOOGLE_API_KEY from env when GOOGLE_GENAI_USE_VERTEXAI is true and no key provided', () => { - process.env['GOOGLE_GENAI_USE_VERTEXAI'] = 'true'; - process.env['GOOGLE_API_KEY'] = 'env-api-key'; + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', 'true'); + vi.stubEnv('GOOGLE_API_KEY', 'env-api-key'); const result = getExpressModeApiKey(); expect(result).toBe('env-api-key'); }); it('should return undefined when GOOGLE_GENAI_USE_VERTEXAI is true but no key available', () => { - process.env['GOOGLE_GENAI_USE_VERTEXAI'] = 'true'; - delete process.env['GOOGLE_API_KEY']; + vi.stubEnv('GOOGLE_GENAI_USE_VERTEXAI', 'true'); + vi.stubEnv('GOOGLE_API_KEY', undefined); const result = getExpressModeApiKey(); expect(result).toBeUndefined(); }); From ae2dcbdee28ead0d4b2d38bb3d6d8c22f9175e93 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Wed, 29 Jul 2026 19:43:53 -0700 Subject: [PATCH 3/3] test: cover the ambient env reads in the dev CLI and telemetry setup Nothing pinned dev/src/cli/cli.ts's DATABASE_URL fallback or dev/src/utils/telemetry_utils.ts's four OTEL_EXPORTER_OTLP_* reads, so a developer or CI runner exporting either silently got a different session service or a different telemetry branch and the suite stayed green. Both suites stub the variables they read, including to undefined for the not-set cases, so they produce the same result on a clean machine and a polluted one. --- dev/test/cli/cli_test.ts | 58 +++++++++++- dev/test/utils/telemetry_utils_test.ts | 122 +++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 dev/test/utils/telemetry_utils_test.ts diff --git a/dev/test/cli/cli_test.ts b/dev/test/cli/cli_test.ts index db3249294..5f5f5bbc2 100644 --- a/dev/test/cli/cli_test.ts +++ b/dev/test/cli/cli_test.ts @@ -4,7 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {LogLevel, setLogLevel} from '@google/adk'; +import { + DatabaseSessionService, + InMemorySessionService, + LogLevel, + setLogLevel, +} from '@google/adk'; import {afterEach, beforeEach, describe, expect, it, Mock, vi} from 'vitest'; import {createProgram} from '../../src/cli/cli.js'; import {createAgent} from '../../src/cli/cli_create.js'; @@ -169,6 +174,57 @@ describe('CLI Entrypoint', () => { }); }); + describe('session service resolution', () => { + const DATABASE_URI = 'postgresql://user:pass@localhost:5432/adk'; + + // Stubbing DATABASE_URL in every case (including to `undefined`) is what + // keeps these assertions true on a machine that exports it. + it('should default to an in-memory session service when DATABASE_URL is unset', async () => { + vi.stubEnv('DATABASE_URL', undefined); + + await parse(['web']); + + const args = vi.mocked(AdkApiServer).mock.calls[0][0]; + expect(args.sessionService).toBeInstanceOf(InMemorySessionService); + }); + + it('should fall back to DATABASE_URL when --session_service_uri is absent', async () => { + vi.stubEnv('DATABASE_URL', DATABASE_URI); + + await parse(['web']); + + const args = vi.mocked(AdkApiServer).mock.calls[0][0]; + expect(args.sessionService).toBeInstanceOf(DatabaseSessionService); + }); + + it('should prefer --session_service_uri over DATABASE_URL', async () => { + vi.stubEnv('DATABASE_URL', DATABASE_URI); + + await parse(['web', '--session_service_uri', 'memory://']); + + const args = vi.mocked(AdkApiServer).mock.calls[0][0]; + expect(args.sessionService).toBeInstanceOf(InMemorySessionService); + }); + + it('should fall back to DATABASE_URL for api_server', async () => { + vi.stubEnv('DATABASE_URL', DATABASE_URI); + + await parse(['api_server']); + + const args = vi.mocked(AdkApiServer).mock.calls[0][0]; + expect(args.sessionService).toBeInstanceOf(DatabaseSessionService); + }); + + it('should fall back to DATABASE_URL for run', async () => { + vi.stubEnv('DATABASE_URL', DATABASE_URI); + + await parse(['run', 'agent.ts']); + + const args = vi.mocked(runAgent).mock.calls[0][0]; + expect(args.sessionService).toBeInstanceOf(DatabaseSessionService); + }); + }); + describe('command: create', () => { it('should call createAgent with default args', async () => { await parse(['create']); diff --git a/dev/test/utils/telemetry_utils_test.ts b/dev/test/utils/telemetry_utils_test.ts new file mode 100644 index 000000000..86e1e198f --- /dev/null +++ b/dev/test/utils/telemetry_utils_test.ts @@ -0,0 +1,122 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + getGcpExporters, + getGcpResource, + maybeSetOtelProviders, + OTelHooks, +} from '@google/adk'; +import {emptyResource} from '@opentelemetry/resources'; +import {SpanProcessor} from '@opentelemetry/sdk-trace-base'; +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {setupTelemetry} from '../../src/utils/telemetry_utils.js'; + +vi.mock('@google/adk', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getGcpExporters: vi.fn(), + getGcpResource: vi.fn(), + maybeSetOtelProviders: vi.fn(), + }; +}); + +const OTEL_ENDPOINT_ENV_VARS = [ + 'OTEL_EXPORTER_OTLP_ENDPOINT', + 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', + 'OTEL_EXPORTER_OTLP_METRICS_ENDPOINT', + 'OTEL_EXPORTER_OTLP_LOGS_ENDPOINT', +] as const; + +const ENDPOINT = 'http://localhost:4318'; + +const spanProcessor: SpanProcessor = { + forceFlush: async () => {}, + onStart: () => {}, + onEnd: () => {}, + shutdown: async () => {}, +}; + +describe('setupTelemetry', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Every case below distinguishes the env branch from the default branch, so + // none of the four endpoint variables may leak in from the environment. + for (const name of OTEL_ENDPOINT_ENV_VARS) { + vi.stubEnv(name, undefined); + } + }); + + it('should wrap the internal exporters in a hook when no OTEL endpoint is set', async () => { + await setupTelemetry(); + + expect(maybeSetOtelProviders).toHaveBeenCalledWith([{spanProcessors: []}]); + }); + + it.each(OTEL_ENDPOINT_ENV_VARS)( + 'should add no hooks of its own when %s is set', + async (name) => { + vi.stubEnv(name, ENDPOINT); + + await setupTelemetry(false, []); + + // The env branch only contributes a hook when there are internal + // exporters to wrap; an empty array is what distinguishes it from the + // default branch. + expect(maybeSetOtelProviders).toHaveBeenCalledWith([]); + }, + ); + + it('should still wrap internal exporters on the env branch', async () => { + vi.stubEnv('OTEL_EXPORTER_OTLP_ENDPOINT', ENDPOINT); + + await setupTelemetry(false, [spanProcessor]); + + expect(maybeSetOtelProviders).toHaveBeenCalledWith([ + {spanProcessors: [spanProcessor]}, + ]); + }); + + it('should treat an empty endpoint variable as unset', async () => { + vi.stubEnv('OTEL_EXPORTER_OTLP_ENDPOINT', ''); + + await setupTelemetry(false, []); + + expect(maybeSetOtelProviders).toHaveBeenCalledWith([{spanProcessors: []}]); + }); + + it('should prefer the GCP branch over the env branch', async () => { + const gcpHooks: OTelHooks = {spanProcessors: []}; + const resource = emptyResource(); + vi.mocked(getGcpExporters).mockResolvedValue(gcpHooks); + vi.mocked(getGcpResource).mockReturnValue(resource); + vi.stubEnv('OTEL_EXPORTER_OTLP_ENDPOINT', ENDPOINT); + + await setupTelemetry(true, []); + + expect(getGcpExporters).toHaveBeenCalledWith({ + enableTracing: true, + enableLogging: false, + enableMetrics: true, + }); + expect(maybeSetOtelProviders).toHaveBeenCalledWith([gcpHooks], resource); + }); + + it('should put the internal exporters ahead of the GCP hooks', async () => { + const gcpHooks: OTelHooks = {spanProcessors: []}; + const resource = emptyResource(); + vi.mocked(getGcpExporters).mockResolvedValue(gcpHooks); + vi.mocked(getGcpResource).mockReturnValue(resource); + + await setupTelemetry(true, [spanProcessor]); + + expect(maybeSetOtelProviders).toHaveBeenCalledWith( + [{spanProcessors: [spanProcessor]}, gcpHooks], + resource, + ); + }); +});