From 36ded926c8f4ad0a33c227c2069434dfec63d805 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Thu, 6 Aug 2026 07:10:48 -0700 Subject: [PATCH 1/3] Test: add a bounded, awaited go mod tidy helper for the cross-language suites The two Go test helpers bootstrap their modules with a synchronous, unbounded execSync that swallows every failure. Add ensureGoModules, which awaits the tidy under an explicit budget and rejects with the command, the module directory, the concrete cause and the manual remediation. --- tests/cross_language/go_modules.ts | 85 +++++++++++++ tests/cross_language/go_modules_test.ts | 156 ++++++++++++++++++++++++ 2 files changed, 241 insertions(+) create mode 100644 tests/cross_language/go_modules.ts create mode 100644 tests/cross_language/go_modules_test.ts diff --git a/tests/cross_language/go_modules.ts b/tests/cross_language/go_modules.ts new file mode 100644 index 000000000..c1a79fdcd --- /dev/null +++ b/tests/cross_language/go_modules.ts @@ -0,0 +1,85 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {execFile, type ExecFileException} from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import {promisify} from 'node:util'; + +const execFileAsync = promisify(execFile); + +/** + * Budget (ms) for the one-off `go mod tidy` that populates a module's `go.sum`. + * + * A cold tidy of the two cross-language Go modules measured 3.2s against an + * empty module cache. 45000 is about 14x that, which leaves room for two Vitest + * workers that contend on the shared module cache. It stays below the 60000 + * per-hook and per-test budget of the cross-language suites, so a wedged + * download reports the message from {@link ensureGoModules} rather than a + * generic Vitest timeout. + */ +export const GO_MOD_TIDY_TIMEOUT_MS = 45000; + +/** The fields of an `execFile` rejection that classify a failure. */ +type ExecFileFailure = Pick; + +function isExecFileFailure(error: unknown): error is ExecFileFailure { + return typeof error === 'object' && error !== null; +} + +function describeFailure(error: unknown, timeoutMs: number): string { + if (!isExecFileFailure(error)) { + return String(error); + } + if (error.code === 'ENOENT') { + return "the 'go' executable was not found on PATH"; + } + if (error.killed) { + return `it exceeded its ${timeoutMs}ms budget`; + } + const stderr = error.stderr?.trim(); + return stderr + ? `it exited with code ${error.code}\n${stderr}` + : `it exited with code ${error.code}`; +} + +/** + * Populates `moduleDir`'s `go.sum` if it is absent. `go.sum` is gitignored, so + * a fresh clone never has one. + * + * This is a no-op when `go.sum` already exists, which is always the case on CI: + * the cross-language workflow tidies both modules in a dedicated step. + * + * @param moduleDir Absolute path of a directory that contains a `go.mod`. + * @param timeoutMs Budget for the tidy. Defaults to + * {@link GO_MOD_TIDY_TIMEOUT_MS}. + * @throws Error naming the module directory and the concrete cause: a missing + * `go` executable, a non-zero exit, or the timeout. Nothing is swallowed, + * so the caller never spawns `go run .` against an unresolved module. + */ +export async function ensureGoModules( + moduleDir: string, + timeoutMs: number = GO_MOD_TIDY_TIMEOUT_MS, +): Promise { + if (fs.existsSync(path.join(moduleDir, 'go.sum'))) { + return; + } + + try { + await execFileAsync('go', ['mod', 'tidy'], { + cwd: moduleDir, + timeout: timeoutMs, + }); + } catch (error: unknown) { + throw new Error( + `go mod tidy failed in ${moduleDir}: ` + + `${describeFailure(error, timeoutMs)}. ` + + `Install the Go toolchain (https://go.dev/dl/) or run 'go mod tidy' ` + + `in that directory manually before running the cross-language tests.`, + {cause: error}, + ); + } +} diff --git a/tests/cross_language/go_modules_test.ts b/tests/cross_language/go_modules_test.ts new file mode 100644 index 000000000..5aaaf5f79 --- /dev/null +++ b/tests/cross_language/go_modules_test.ts @@ -0,0 +1,156 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type {ExecFileException, ExecFileOptions} from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {ensureGoModules, GO_MOD_TIDY_TIMEOUT_MS} from './go_modules.js'; + +type ExecFileCallback = ( + error: ExecFileException | null, + stdout: string, + stderr: string, +) => void; + +/** + * `promisify` only uses a function's `util.promisify.custom` implementation, + * which this replacement does not carry, so it falls back to the generic + * callback path. The mock must therefore call its last argument. + */ +const execFileMock = vi.hoisted(() => + vi.fn< + ( + file: string, + args: readonly string[], + options: ExecFileOptions, + callback: ExecFileCallback, + ) => void + >(), +); + +vi.mock('node:child_process', async (importOriginal) => ({ + ...(await importOriginal()), + execFile: execFileMock, +})); + +/** Fails the tidy with `error`, as `execFile`'s callback would. */ +function rejectWith(error: ExecFileException): void { + execFileMock.mockImplementation((_file, _args, _options, callback) => { + callback(error, '', ''); + }); +} + +describe('ensureGoModules', () => { + let moduleDir: string; + + beforeEach(() => { + moduleDir = fs.mkdtempSync(path.join(os.tmpdir(), 'adk-go-modules-')); + execFileMock.mockImplementation((_file, _args, _options, callback) => { + callback(null, '', ''); + }); + }); + + afterEach(() => { + fs.rmSync(moduleDir, {recursive: true, force: true}); + execFileMock.mockReset(); + }); + + it('does not tidy when go.sum already exists', async () => { + fs.writeFileSync(path.join(moduleDir, 'go.sum'), ''); + + await ensureGoModules(moduleDir); + + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('tidies the module directory when go.sum is absent', async () => { + await ensureGoModules(moduleDir); + + expect(execFileMock).toHaveBeenCalledWith( + 'go', + ['mod', 'tidy'], + expect.objectContaining({ + cwd: moduleDir, + timeout: GO_MOD_TIDY_TIMEOUT_MS, + }), + expect.any(Function), + ); + }); + + it('rejects by name when the go executable is missing', async () => { + rejectWith(Object.assign(new Error('spawn go ENOENT'), {code: 'ENOENT'})); + + await expect(ensureGoModules(moduleDir)).rejects.toThrow( + `go mod tidy failed in ${moduleDir}: the 'go' executable was not found ` + + `on PATH. Install the Go toolchain (https://go.dev/dl/) or run ` + + `'go mod tidy' in that directory manually before running the ` + + `cross-language tests.`, + ); + }); + + it('rejects with the budget it was given when the tidy times out', async () => { + rejectWith(Object.assign(new Error('killed'), {killed: true, code: null})); + + await expect(ensureGoModules(moduleDir, 1234)).rejects.toThrow( + 'it exceeded its 1234ms budget', + ); + expect(execFileMock).toHaveBeenCalledWith( + 'go', + ['mod', 'tidy'], + expect.objectContaining({timeout: 1234}), + expect.any(Function), + ); + }); + + it('reports stderr when the tidy exits non-zero', async () => { + rejectWith( + Object.assign(new Error('exit 1'), { + code: 1, + stderr: 'go: some/module@v1: invalid version\n', + }), + ); + + await expect(ensureGoModules(moduleDir)).rejects.toThrow( + 'it exited with code 1\ngo: some/module@v1: invalid version.', + ); + }); + + it('omits the stderr line when the failed tidy wrote nothing', async () => { + rejectWith(Object.assign(new Error('exit 2'), {code: 2, stderr: ''})); + + await expect(ensureGoModules(moduleDir)).rejects.toThrow( + 'it exited with code 2. Install the Go toolchain', + ); + }); + + it('stringifies a rejection that is not an object', async () => { + execFileMock.mockImplementation(() => { + throw 'go exploded'; + }); + + await expect(ensureGoModules(moduleDir)).rejects.toThrow( + `go mod tidy failed in ${moduleDir}: go exploded.`, + ); + }); + + it('keeps the original failure as the error cause', async () => { + const original = Object.assign(new Error('spawn go ENOENT'), { + code: 'ENOENT', + }); + rejectWith(original); + + const rejection = await ensureGoModules(moduleDir).catch( + (error: unknown) => error, + ); + + if (!(rejection instanceof Error)) { + expect.fail(`expected a rejection, got ${String(rejection)}`); + } + expect(rejection.cause).toBe(original); + }); +}); From 083ae0c00020696d1b5505eeab84c6f764b847b4 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Thu, 6 Aug 2026 07:10:56 -0700 Subject: [PATCH 2/3] Fix: await the Go module bootstrap and stop swallowing its failures AdkGoServer and GoAgent now call ensureGoModules, so a missing Go toolchain fails in milliseconds with a Go-specific message instead of a hook or test timeout. Raise AdkGoServer's default readiness budget to 60000 to match the Node API server: `go run .` compiles the module before it boots, so the tighter default was backwards. --- .../a2a/go_ts/go_client/go_agent.ts | 17 ++--------- .../a2a/ts_go/go_backend/go_server.ts | 30 ++++++++----------- 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/tests/cross_language/a2a/go_ts/go_client/go_agent.ts b/tests/cross_language/a2a/go_ts/go_client/go_agent.ts index 288ed3f55..bb456505a 100644 --- a/tests/cross_language/a2a/go_ts/go_client/go_agent.ts +++ b/tests/cross_language/a2a/go_ts/go_client/go_agent.ts @@ -5,10 +5,9 @@ */ import type {Event} from '@google/adk'; -import {execSync, spawn} from 'node:child_process'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; +import {spawn} from 'node:child_process'; import * as readline from 'node:readline'; +import {ensureGoModules} from '../../../go_modules.js'; export interface GoAgentParams { dir: string; @@ -28,17 +27,7 @@ export class GoAgent { } public async *run(userMessage: string): AsyncGenerator { - if (!fs.existsSync(path.join(this.dir, 'go.sum'))) { - try { - execSync('go mod tidy', { - cwd: this.dir, - stdio: 'inherit', - env: process.env, - }); - } catch (_e: unknown) { - console.warn('Failed to run go mod tidy'); - } - } + await ensureGoModules(this.dir); const child = spawn( 'go', diff --git a/tests/cross_language/a2a/ts_go/go_backend/go_server.ts b/tests/cross_language/a2a/ts_go/go_backend/go_server.ts index cf25a2751..353629f31 100644 --- a/tests/cross_language/a2a/ts_go/go_backend/go_server.ts +++ b/tests/cross_language/a2a/ts_go/go_backend/go_server.ts @@ -4,10 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {execSync, spawn} from 'node:child_process'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; +import {spawn} from 'node:child_process'; import {BaseTestServer} from '../../../../integration/test_case_utils.js'; +import {ensureGoModules} from '../../../go_modules.js'; /** * Interface representing the parameters for creating the test Go agent server. @@ -18,7 +17,15 @@ export interface TestGoServerParams { startFailureTimeout?: number; } -const DEFAULT_TIMEOUT = 30000; +/** + * Readiness budget (ms) for the Go server. + * + * `go run .` compiles the module from source before the server prints its + * banner, which is strictly more work than the already-built Node API server + * does at start-up. This default therefore matches that server's 60000 rather + * than undercutting it. + */ +const DEFAULT_TIMEOUT = 60000; /** * Go server for testing. @@ -32,20 +39,7 @@ export class AdkGoServer extends BaseTestServer { } async start(): Promise { - if (!fs.existsSync(path.join(this.params.serverDir, 'go.sum'))) { - try { - console.log('Running go mod tidy to fetch dependencies...'); - execSync('go mod tidy', { - cwd: this.params.serverDir, - stdio: 'inherit', - env: process.env, - }); - } catch (_e: unknown) { - console.warn( - 'Failed to run go mod tidy, ensure go is installed and network is available.', - ); - } - } + await ensureGoModules(this.params.serverDir); await this.startProcess({ spawnProcess: () => { From 211b922cfcdadcb1a56f141cf3b7526daefe312d Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Thu, 6 Aug 2026 07:55:59 -0700 Subject: [PATCH 3/3] Refactor: drop the unused tidy budget parameter and shrink two doc comments No caller sets the timeout, so ensureGoModules now reads GO_MOD_TIDY_TIMEOUT_MS directly. Inline the single-use ExecFileFailure alias into its guard, and cut the DEFAULT_TIMEOUT and budget rationales to the reason each number holds. --- .../a2a/ts_go/go_backend/go_server.ts | 8 ++--- tests/cross_language/go_modules.ts | 31 ++++++------------- tests/cross_language/go_modules_test.ts | 12 ++----- 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/tests/cross_language/a2a/ts_go/go_backend/go_server.ts b/tests/cross_language/a2a/ts_go/go_backend/go_server.ts index 353629f31..12a92651c 100644 --- a/tests/cross_language/a2a/ts_go/go_backend/go_server.ts +++ b/tests/cross_language/a2a/ts_go/go_backend/go_server.ts @@ -18,12 +18,8 @@ export interface TestGoServerParams { } /** - * Readiness budget (ms) for the Go server. - * - * `go run .` compiles the module from source before the server prints its - * banner, which is strictly more work than the already-built Node API server - * does at start-up. This default therefore matches that server's 60000 rather - * than undercutting it. + * Readiness budget (ms). Matches `AdkTsApiServer`'s 60000 because `go run .` + * compiles the module before the server boots. */ const DEFAULT_TIMEOUT = 60000; diff --git a/tests/cross_language/go_modules.ts b/tests/cross_language/go_modules.ts index c1a79fdcd..6425f1e55 100644 --- a/tests/cross_language/go_modules.ts +++ b/tests/cross_language/go_modules.ts @@ -13,24 +13,19 @@ const execFileAsync = promisify(execFile); /** * Budget (ms) for the one-off `go mod tidy` that populates a module's `go.sum`. - * - * A cold tidy of the two cross-language Go modules measured 3.2s against an - * empty module cache. 45000 is about 14x that, which leaves room for two Vitest - * workers that contend on the shared module cache. It stays below the 60000 - * per-hook and per-test budget of the cross-language suites, so a wedged - * download reports the message from {@link ensureGoModules} rather than a + * A cold tidy measured 3.2s; 45000 stays under the 60000 per-hook and per-test + * budget of the cross-language suites, so this helper's message wins over a * generic Vitest timeout. */ export const GO_MOD_TIDY_TIMEOUT_MS = 45000; -/** The fields of an `execFile` rejection that classify a failure. */ -type ExecFileFailure = Pick; - -function isExecFileFailure(error: unknown): error is ExecFileFailure { +function isExecFileFailure( + error: unknown, +): error is Pick { return typeof error === 'object' && error !== null; } -function describeFailure(error: unknown, timeoutMs: number): string { +function describeFailure(error: unknown): string { if (!isExecFileFailure(error)) { return String(error); } @@ -38,7 +33,7 @@ function describeFailure(error: unknown, timeoutMs: number): string { return "the 'go' executable was not found on PATH"; } if (error.killed) { - return `it exceeded its ${timeoutMs}ms budget`; + return `it exceeded its ${GO_MOD_TIDY_TIMEOUT_MS}ms budget`; } const stderr = error.stderr?.trim(); return stderr @@ -54,16 +49,11 @@ function describeFailure(error: unknown, timeoutMs: number): string { * the cross-language workflow tidies both modules in a dedicated step. * * @param moduleDir Absolute path of a directory that contains a `go.mod`. - * @param timeoutMs Budget for the tidy. Defaults to - * {@link GO_MOD_TIDY_TIMEOUT_MS}. * @throws Error naming the module directory and the concrete cause: a missing * `go` executable, a non-zero exit, or the timeout. Nothing is swallowed, * so the caller never spawns `go run .` against an unresolved module. */ -export async function ensureGoModules( - moduleDir: string, - timeoutMs: number = GO_MOD_TIDY_TIMEOUT_MS, -): Promise { +export async function ensureGoModules(moduleDir: string): Promise { if (fs.existsSync(path.join(moduleDir, 'go.sum'))) { return; } @@ -71,12 +61,11 @@ export async function ensureGoModules( try { await execFileAsync('go', ['mod', 'tidy'], { cwd: moduleDir, - timeout: timeoutMs, + timeout: GO_MOD_TIDY_TIMEOUT_MS, }); } catch (error: unknown) { throw new Error( - `go mod tidy failed in ${moduleDir}: ` + - `${describeFailure(error, timeoutMs)}. ` + + `go mod tidy failed in ${moduleDir}: ${describeFailure(error)}. ` + `Install the Go toolchain (https://go.dev/dl/) or run 'go mod tidy' ` + `in that directory manually before running the cross-language tests.`, {cause: error}, diff --git a/tests/cross_language/go_modules_test.ts b/tests/cross_language/go_modules_test.ts index 5aaaf5f79..84fd7d68b 100644 --- a/tests/cross_language/go_modules_test.ts +++ b/tests/cross_language/go_modules_test.ts @@ -93,17 +93,11 @@ describe('ensureGoModules', () => { ); }); - it('rejects with the budget it was given when the tidy times out', async () => { + it('names the budget when the tidy times out', async () => { rejectWith(Object.assign(new Error('killed'), {killed: true, code: null})); - await expect(ensureGoModules(moduleDir, 1234)).rejects.toThrow( - 'it exceeded its 1234ms budget', - ); - expect(execFileMock).toHaveBeenCalledWith( - 'go', - ['mod', 'tidy'], - expect.objectContaining({timeout: 1234}), - expect.any(Function), + await expect(ensureGoModules(moduleDir)).rejects.toThrow( + `it exceeded its ${GO_MOD_TIDY_TIMEOUT_MS}ms budget`, ); });