Fix: import Client from the @google-cloud/vertexai package root in two integration tests - #622
Open
AmaadMartin wants to merge 1 commit into
Open
Fix: import Client from the @google-cloud/vertexai package root in two integration tests#622AmaadMartin wants to merge 1 commit into
AmaadMartin wants to merge 1 commit into
Conversation
…o integration tests Both tests reached into the dependency's compiled output (@google-cloud/vertexai/build/src/genai/client.js) for a class the package already re-exports from its root entry point. build/src/... is emitted output, not documented surface: an added exports map, an outDir change or a move of genai/client.ts would break both files even though the symbol never moved. The root barrel re-exports the identical class object, so this is a no-op at both the type and runtime level. Seven other call sites in the repo already use the root specifier, so this also removes an internal inconsistency. Sessions and the genai/types symbols are deliberately left on their deep specifiers: the root barrel does not re-export them, so hoisting them is a TS2305 compile error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A — no public issue is open for this.
Problem: Two integration tests import the
Clientclass by reaching into the@google-cloud/vertexaipackage's compiled output directory:build/src/...is emitted output, not documented surface. Nothing guarantees that path across releases — if the package adds anexportsmap, changes itsoutDir, or movesgenai/client.ts, both files break with a module-resolution error even though the symbol they want never moved. Seven other call sites in this repo already importClientfrom the package root (core/src/memory/vertex_ai_memory_bank_service.ts,core/src/code_executors/agent_engine_sandbox_code_executor.ts,core/src/skills/gcp_skill_registry.ts,core/test/memory/vertex_ai_memory_bank_service_test.ts,core/test/code_executors/agent_engine_sandbox_code_executor_test.ts,core/test/tools/skills/skill_registry_test.ts,tests/integration/agents/agent_with_sandbox_executor_test.ts), so these two lines are also an internal inconsistency.Solution: Point both at the package root,
import {Client} from '@google-cloud/vertexai';. This is a test-only, behaviour-preserving change — no production code, no public API, no dependency versions.Why it is safe. The installed version is
@google-cloud/vertexai@1.12.0(core/package.jsondeclares^1.12.0; the lockfile pins 1.12.0). Its root barrel re-exports the class directly —node_modules/@google-cloud/vertexai/build/src/index.d.ts:17, verbatim:and
build/src/index.jsre-exports it as a live getter onto the same binding, so the root and deep specifiers name the identical class object. Verified at runtime:Both tests use
Clientpurely as a type (anas unknown as Clientcast over a hand-rolled object injected through the service constructor); neither file instantiates it, and neither contains avi.mock, so module resolution at test time is not involved at all.Why the sibling deep imports were deliberately left alone. The root barrel's
export * from './types'refers tobuild/src/types/— the legacy Vertex AI types module — which is a different module frombuild/src/genai/types.Sessions(./genai/sessions) is likewise absent from the root barrel. Hoisting them is therefore a compile error, not a style choice. Demonstrated by mutating line 8 of the agent-tool test to the root specifier:So
Clientis the only symbol in these files that is hoistable. Left untouched:tests/integration/tools/agent_tool_vertexai_test.ts:8andtests/integration/sessions/vertex_ai_session_service_test.ts:7(bothSessions), and every deep import undercore/src,dev/srcanddev/test. In particulardev/test/cli/cli_deploy_agent_engine_test.tsdoesvi.mock('@google-cloud/vertexai/build/src/genai/client.js', ...)paired with anew Client({...})indev/src/cli/deploy/cli_deploy_agent_engine.ts— those useClientas a value and the mock is bound to the deep specifier, which is a materially riskier change that must not ride along here.Overlap check with open PRs (disclosure). Before implementing I checked the 521 open PRs on the fork for a collision. Two open PRs already contain these same two import edits as part of larger changes: #475 (Chore(lint): forbid deep imports into the @google-cloud/vertexai build output — same rename plus an ESLint guard and
core/devsource changes) and #303 (Fix: contain @google-cloud/vertexai build-output deep imports behind one adapter module — same rename plus a newcore/src/utils/vertex_ai_internal.tsadapter). Those two conflict with each other on these lines and each couples the rename to a broader design decision. This PR is the standalone two-line subset, which can land regardless of which of the larger designs is chosen; if either #475 or #303 merges first, this PR becomes a no-op and should simply be closed.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
No new tests, deliberately. This change adds zero lines of production code and alters no behaviour, so there is nothing new to cover and no assertion that could distinguish before from after — a new test here would be a green light with no signal. The repo's "Add a New Test; Do Not Rewrite an Existing One" rule also means no existing test was touched: no
describe,it,expect, mock object or fixture changed, only the import specifier. The diff is 2 files / 2 insertions / 2 deletions.The analogue of "prove the test can fail" for a no-op rename is a before/after baseline plus a negative control, both run:
Baseline, on the unmodified tree:
After the edit — identical pass/fail/skip counts:
Both tests genuinely execute — 0 skipped, no
skipIf, noprocess.envread, no network I/O. The model isGeminiWithMockResponsesand the Vertex AI clients are plain injected object literals.Negative control — proof that the checks would catch a bad hoist rather than passing vacuously. Mutating line 8 of
tests/integration/tools/agent_tool_vertexai_test.tstoimport {Sessions} from '@google-cloud/vertexai';makes the type check fail:This confirms
tsc --noEmitreally does type-checktests/integration/**(the roottsconfig.jsondeclares noinclude/files, so the default applies), so the green run below is real signal about theClientspecifier. The mutation was reverted.Type check.
npm run ts:checkoutput is byte-identical before and after the change (diffof the two captured runs is empty):Found 281 errors in 41 filesboth times — a pre-existing backlog onmain, none of it in either file touched here. Zero new diagnostics.Lint and formatting:
prettier-plugin-organize-importsleaves both import blocks in place:'@google-cloud/vertexai'is a proper prefix of'@google-cloud/vertexai/build/src/genai/sessions.js'so it still sorts before line 8, and it sorts before'@google/adk'because-(0x2D) precedes/(0x2F).Scope guard:
[x] All unit tests pass locally.
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
From the repository root:
Checks 2, 3 and 4 were run and pass; 3 prints
17:export { Client } from './genai/client';and 4 prints nothing.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.
CI
All jobs green on
run-tests: ubuntu-latest, macos-latest, windows-latest, plus the standalonerun-testsjob.The first windows-latest attempt failed with
Error: CLI exited prematurely with code 1attests/integration/test_case_utils.ts:341intests/integration/a2a/stream/stream_test.ts. That is the spawned-test-server harness failing to start, not an assertion: the same run reported 2678 tests passed, 44 skipped, 0 tests failed (1 failed suite). It is a known pre-existing Windows flake unrelated to an import specifier, and the job passed on re-run with no code change.