Fix: run the orphaned unit:integrations vitest project from the root test scripts - #236
Fix: run the orphaned unit:integrations vitest project from the root test scripts#236AmaadMartin wants to merge 6 commits into
Conversation
3c40559 to
db1cb0e
Compare
integrations/test/version_test.ts hardcoded `expect(version).toBe('1.3.0')`
while integrations/src/version.ts exports '1.4.0', so the test is currently
red. It went stale unnoticed because the vitest project that owns it
(`unit:integrations`) is invoked by no npm script and no workflow, so the
file has never actually run.
Bumping the literal to '1.4.0' would only re-rot on the next release: the
release automation rewrites integrations/src/version.ts (via the
x-release-please-version annotation) but never touches test literals. Assert
against the version declared in integrations/package.json instead. Both
sides are updated in the same release commit, so the assertion is
self-maintaining, and it guards the invariant actually worth guarding: the
exported constant must not drift from the published package version.
vitest.config.ts declares a `unit:integrations` project owning integrations/test/**/*_test.ts, but the name appeared nowhere else in the repository: no npm script and no workflow invoked it, so those tests never ran. validation.yaml runs `npm run test:coverage` and cross-language-integration.yml runs `npm run test:cross-language`, which between them reached every project except this one. Add `--project unit:integrations` to `test`, `test:unit` and `test:coverage`, positioned after `unit:dev` to match the declaration order in vitest.config.ts. No `test:integrations` script is added on purpose: it would sit one character from the existing `test:integration` (which runs the unrelated `integration` project over tests/integration/) and invite mistakes. `unit:core` and `unit:dev` have no individual scripts either. The coverage thresholds are deliberately left untouched. coverage.include already lists integrations/src/**/*.ts and coverage.all defaults to true, so those files were already in the denominator scored at 0%; running the project only adds to the numerator. Measured over `unit:core + unit:dev` (v8), All files goes 88.94/88.11/89.58/88.94 to 88.95/88.14/89.73/88.95 statements/branches/functions/lines - every metric up.
8136fe9 to
a2c057a
Compare
… projects The two project names differ by one character and by scope: unit:integrations owns the integrations/ workspace package, integration owns the cross-component suite in tests/integration/. That similarity is part of why the former was overlooked by the root test scripts for three releases. Comments only; no project definition, glob, or threshold changes.
|
Independent verification from a duplicate task that was elaborated against the same three defects and is being closed in favour of this PR. Recording the measurements here so they are not lost — all run locally against this branch's exact two-file state ( Postconditions
The only failure in the Mutation proof, both drift directions — the assertion fails when it should:
Anti-rot — bumping both Two things a reviewer might otherwise ask for, which the evidence says not to:
|
The disambiguation between unit:integrations and integration was written twice, once from each side, and each copy restated the include glob two lines below it. Keep the comment on unit:integrations -- the project this change wires in, and the less obvious of the two -- and drop the mirror.
The approved spec scopes this change to the three root test-script strings in package.json and the integrations version test, touching vitest.config.ts "at most" for the coverage thresholds block -- whose expected outcome is no edit at all. The project-naming comment added earlier sits outside that ceiling, so it is removed and the diff is now exactly the two files the fix requires. No behaviour change: the comment never affected project resolution.
integrations/build.js compiles src/index.ts and src/index_web.ts into separate published artifacts -- dist/esm and dist/cjs from the first, the dist/web bundle the package's browser field points at from the second -- so an export added to one entry point and forgotten in the other ships a browser bundle silently missing the symbol. Nothing guarded that, and index_web.ts was the one integrations source file no test reached, sitting at 0% coverage even after the project was wired into the root scripts. Assert the two entry points expose the same export names and the same version binding. The key-set assertion is guarded against passing vacuously if both entry points ever resolve to nothing.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
No existing issue. The gap originates in google#449, the PR that added the
integrationspackage together with theunit:integrationsvitest project.Problem:
vitest.config.tsdeclares six projects, but the root test scripts selected only five.unit:integrations— the project that ownsintegrations/test/**/*_test.ts(vitest.config.ts:51) — was selected by nothing: no npm script, no workflow. The string appeared exactly once in the whole repository, in its own declaration:.github/workflows/validation.yaml:41runsnpm run test:coverage, and.github/workflows/cross-language-integration.ymlrunsnpm run test:cross-language; between them every project reached CI except this one. Sointegrations/test/had never executed — not in CI, and not for a contributor followingCONTRIBUTING.md'snpm run build && npm test.The blind spot had already rotted the one test living there.
integrations/test/version_test.tsassertedexpect(version).toBe('1.3.0')whileintegrations/src/version.ts:8exports'1.4.0'at this branch's base. The drift is easy to trace: the test was introduced asserting'1.3.0'in google#449, and the later v1.4.0 release bumpedintegrations/src/version.tsthrough release-please'sextra-filesentry inrelease-please-config.json. release-please does not touch test files, and nothing ran the test, so nobody noticed.The gap has since widened by another release, which is the clearest evidence that a literal is the wrong fix:
maintoday exports'1.5.0'fromintegrations/src/version.tswhile that file's assertion is still'1.3.0'— two releases stale. The derived assertion shipped here is unaffected by that, and was verified against it directly (durability proof 4 below).The next re-rot is not hypothetical either. google#586 (
Release: v1.6.0, branchrelease-please--branches--main) is open right now, and its diff moves bothintegrations/src/version.tsandintegrations/package.jsonfrom1.5.0to1.6.0while touchingintegrations/test/version_test.tsnot at all:So re-pinning the literal would turn that release PR red the moment it rebases — the identical breakage, except this time it blocks a release. Because the assertion is derived from the manifest google#586 bumps in the same commit, it survives untouched; durability proof 4 below runs exactly that
1.6.0/1.6.0pair.That is why the script change cannot land on its own — it would turn CI red. Both halves have to ship together.
Solution: three changes, in three files.
Make the assertion self-maintaining (
integrations/test/version_test.ts). Bumping the literal to'1.4.0'would re-rot at the very next release, and the breakage would then surface inside an automated release PR, where it is maximally disruptive. Instead the test asserts that the exported constant equals the version declared inintegrations/package.json. Thelinked-versionsplugin bumps both sides in the same release commit, so the assertion cannot go stale, and it pins the invariant actually worth pinning: the exportedversionmust never drift from the published package version.Note this is a real drift guard, not a tautology: the two files are updated by two independent release-please paths —
integrations/src/version.tsby thegenericextra-filesupdater keyed on the// x-release-please-versionmarker, andintegrations/package.jsonby thenoderelease type. Nothing but this test ties them together.Wire the project in (
package.json).--project unit:integrationsis added totest,test:unitandtest:coverage, positioned afterunit:devso the flag order matches the declaration order invitest.config.ts.Cover the browser entry point (
integrations/test/index_web_test.ts, new). Wiring the project in takesindex.tsandversion.tsfrom 0% to 100%, but leavesindex_web.ts— the one integrations source file no test reaches — still at 0%. That gap is not cosmetic:integrations/build.jscompilessrc/index.tsandsrc/index_web.tsas independent entry points (dist/esm+dist/cjsfrom the first; thedist/webbundle thatintegrations/package.json'sbrowserfield publishes from the second), so an export added to one and forgotten in the other ships a browser bundle silently missing the symbol, with nothing to catch it. The new test asserts the two entry points expose the same export names and the sameversionbinding.Both entry points are imported by relative
../src/*.jspath rather than through@google/adk-integrations. That is forced rather than a style choice:integrations/package.jsondeclares only the"."subpath in itsexportsmap, soindex_webis unreachable by package specifier. It matches the ~101 existing tests that import../../src/<mod>.js, and importing both sides the same way keeps the comparison symmetric with the two inputsbuild.jsactually consumes.The key-set assertion carries an explicit
expect(nodeExports).not.toHaveLength(0)guard. Without it the test would pass vacuously if both entry points ever resolved to an empty namespace — an equality assertion between two empty key lists is always true, which is the shape of a test that stays green forever while guarding nothing.vitest.config.tsis not touched at all. An earlier revision of this branch also added a two-line comment there naming the difference between the similarly-spelledunit:integrationsandintegrationprojects. That comment has been removed: the approved scope for this fix is the three script strings plus the version test, and the config file was in scope only for a coverage-threshold re-baseline that measured out as a no-op (see the coverage section). A complexity review had already flagged the comment as duplication once, cutting it from seven lines to two; rather than keep shrinking an edit the fix does not need, it is gone. The diff is exactly the three files above, andgit diffonvitest.config.tsis empty.**The alternative that was rejected: dropping
integrations/src/**/\*.tsfromcoverage.include.** There are only two ways to make the coverage report honest — run the project, or stop counting the package — and this PR takes the first. Three reasons:unit:integrationsproject already exists invitest.config.ts. feat(integrations): create new top-level integrations package google/adk-js#449 wired the package into the rootworkspacesarray and into the vitest project list and only missed the script flags, so the omission is an oversight, not a policy decision. Fixing the flags restores the intent already expressed in the config.@google/adk-integrationsis a published package that will grow (Feat: port FirestoreSessionService from adk-python to @google/adk-integrations #466 is already queuing aFirestoreSessionServiceinto it, with two test files of its own). Excluding it from the gate now guarantees that everything landing there in future is untested and unmeasured, and the exclusion would be invisible at the moment it starts to matter.coverage.includewould leave a dead vitest project and a never-run, permanently-failing test file behind — strictly worse than today, because it converts visible rot into invisible rot.Two scope notes, both deliberate:
test:unitis updated alongsidetestandtest:coverage, although the defect report names only the latter two.unit:integrationsis a unit project; leaving it out of the unit alias would reproduce the identical bug one script over, and it is a one-flag addition on an adjacent line.cross-languagestays out oftestandtest:coverage, on purpose. It is the other project the root scripts do not select, but its exclusion is legitimate rather than accidental: it needs a Go toolchain and has its own workflow,.github/workflows/cross-language-integration.yml, which runsnpm run test:cross-languageonmacos-latestaftergo mod tidy.unit:integrationshad no such second home — that is exactly what made it orphaned rather than merely deselected.Deliberately not done:
test:integrationsscript would be one character away from the existingtest:integrationand is not needed by anything:test:unitnow covers the project, and anyone wanting it alone can runnpx vitest --project unit:integrations.vitest.config.tschange of any kind. Project definitions, aliases,includeglobs, coverageinclude,globalSetupand the thresholds are all untouched.integrationproject. A rename would churn the project id, thetest:integrationscript name and contributor habits, for a clarity win that is not what this bug is about..github/workflows/validation.yamlchange. It already invokesnpm run test:coverage, so fixing the script is sufficient — the workflow picks the project up for free on all three OS legs.version_test.ts. A secondexpect(version).toMatch(/^\d+\.\d+\.\d+$/)was considered and left out: the invariant worth pinning (exported constant ≡ published version) is already asserted, and a shape check on a string release-please writes adds no failure mode the equality check does not already catch. It is implemented separately in Test: assert the integrations version export is well-formed semver (stacked on #236) #479, stacked on this branch, for anyone who wants it.Test: pin the integrations web entry point to the node public surface) proposed the sameindex_web_test.tsagainst this branch as its base. Output 3 above is now in this PR at review request, which makes Test: pin the integrations web entry point to the node public surface (stacked on #236) #513 a no-op against its own base rather than a competing implementation; that is noted on Test: pin the integrations web entry point to the node public surface (stacked on #236) #513 so its author can close it. Nothing else in the stack is affected — Fix: select vitest unit projects by wildcard so new unit suites cannot be silently excluded #311, Test: fail CI when a vitest project is not run by any npm script #343 and Test: assert the integrations version export is well-formed semver (stacked on #236) #479 touch different lines.Implementation note.
integrations/package.jsonis read with a native JSON module import (import packageJson from '../package.json' with {type: 'json'}) rather thanreadFileSync+fileURLToPath. It is one line instead of six, and it removes manual path handling entirely, so there is nothing to get wrong on thewindows-latestleg. The import attribute is required, not decorative: the roottsconfig.jsonsetsmodule/moduleResolution: nodenext, under which ESM demands the attribute. Verified to typecheck — see the static-gate results below.This is the established idiom in this repository rather than a new pattern:
grep -rn "with {type: 'json'}" --include=*.tsfinds 23 existing import sites (undertests/integration/skills/,tests/integration/streaming/,tests/integration/a2a/,tests/integration/agents/and others). It also needs no compiler-option change —tsc --showConfigreportsresolveJsonModule: truealready resolved from the root config, sotsconfig.jsonis untouched. And it cannot perturb the published build:integrations/tsconfig.jsonsets"include": ["src/**/*"], so the workspace'stsc --emitDeclarationOnlynever sees this test file and norootDirconflict is introduced.The
readFileSync+fileURLToPathalternative was considered and rejected on evidence. The case against the JSON import is that the roottsconfig.jsondoes not spell outresolveJsonModule, so the import would supposedly force a repo-wide widening of the shared config. That premise is false and was measured rather than assumed:TypeScript turns
resolveJsonModuleon by default undermoduleResolution: nodenext, so the widening never happens. With that premise gone, the JSON import is one line against six, and — the part that actually matters under the "no weak types" rule — it needs no cast:packageJson.versionis typedstringfrom the file itself, whereasJSON.parsereturnsanyand has to be narrowed withas {version: string}, an unchecked assertion that would silently survive{"version": 42}. Fewer lines and a stronger type, so the import wins on both counts.Collision check (required before implementation; recorded here either way):
Re-run in full on every revision of this PR (
--limit 1000, not the default page, so older PRs are not silently truncated away), thengh pr diff --name-onlyon every plausibly adjacent hit. No open PR adds--project unit:integrationsto the root scripts, so there is no competing implementation.The check did surface an overlap in the other direction, and it is worth recording how it was resolved. Two additional test artifacts belonging to this fix — the semver assertion on
version_test.tsand the newindex_web_test.ts— had already been implemented in open PRs #479 and #513, both based on this very branch. An earlier revision of this PR therefore left both out to avoid duplicating live work. A correctness review disagreed on the second one, on the grounds thatindex_web.tssitting at 0% coverage leaves the entry-point parity invariant unguarded no matter which PR is nominally responsible, soindex_web_test.tsis now in this PR. That makes #513 a no-op against its own base rather than a rival implementation; the semver assertion stays in #479, which is unaffected. See the stacked-PR bullet below.Fix: Run the unit:integrations vitest project in CI and de-rot its version test #245 (
fix/run-unit-integrations-project-in-ci) was a duplicate of this PR — same two files, equivalent fix. This PR was opened first (2026-07-29T09:55:08Zvs2026-07-29T12:15:53Z, 2h20m later), so it was kept and the duplicate recorded rather than a third competing variant being built. Fix: Run the unit:integrations vitest project in CI and de-rot its version test #245 has since been closed, so only this one remains.Chore: hoist duplicated vitest project alias map into a shared constant #261 and Chore(test): hoist the shared vitest alias map into one constant #434 both hoist the duplicated per-project
aliasmap into one shared constant, and are therefore the only other open PRs touchingvitest.config.ts. Checked rather than assumed: both replace the sixalias: {...}literals and neither adds or edits aname:field or any comment near one, so they overlap this diff in file only, not in lines. (They duplicate each other, which is a separate matter for whoever triages them.)Feat: Split heavy child-process integration suites into a dedicated integration:slow vitest project and CI job #237 (
integration:slowproject) also editspackage.json, but only adds atest:integration-slowline — no overlap with the three lines changed here, though the insertion point is adjacent enough that whichever merges second may need a trivial context resolution.Feat: Typecheck the dev and integrations workspace test trees #229 (typecheck the dev and integrations test trees) adds
integrations/test/tsconfig.json, which would typecheck the file changed here under a stricter per-tree project. Checked for forward compatibility rather than assumed: applying that PR's proposedintegrations/test/tsconfig.jsonlocally and runningnpx tsc -p integrations/test/tsconfig.jsonagainst this branch exits 0, so the JSON import attribute survives it. No stacking needed.Test: add self-maintaining version consistency tests for core and dev #258 (version consistency tests for
coreanddev) is the same idea applied to the other two workspaces and touches disjoint files (core/test/version_test.ts,dev/test/version_test.ts).Test: add a repo-level release version consistency test #297 (repo-level release version consistency test) adds
tests/integration/release/version_consistency_test.tsin theintegrationproject. Checked for overlap rather than assumed: its two assertions pinpackage.json↔.release-please-manifest.jsonagreement across the linked-versions group, and neither readssrc/version.ts. It therefore covers a different invariant than this PR, which pins the exported runtime constant against its own manifest — the drift that actually occurred at the v1.4.0 release. Complementary, disjoint files, no collision.Fix: select vitest unit projects by wildcard so new unit suites cannot be silently excluded #311, Test: fail CI when a vitest project is not run by any npm script #343, Test: assert the integrations version export is well-formed semver (stacked on #236) #479 and Test: pin the integrations web entry point to the node public surface (stacked on #236) #513 are stacked on this branch (
gh pr view --json baseRefNamereportsbase=fix/run-unit-integrations-vitest-projectfor all four), so they are downstream consumers rather than competitors. Fix: select vitest unit projects by wildcard so new unit suites cannot be silently excluded #311 generalizes the three script lines here into a--project 'unit:*'wildcard; Test: fail CI when a vitest project is not run by any npm script #343 addstests/integration/repo_config/vitest_projects_test.ts, a guard that fails CI when any declared vitest project is selected by no npm script — the structural fix for the class of bug this PR fixes by instance; Test: assert the integrations version export is well-formed semver (stacked on #236) #479 adds a secondit()to the very file changed here, asserting the exportedversionis well-formed semver. All build on this change and none can land before it — until these script lines exist, nothing runsintegrations/test/**. This is also why the branch has not been force-pushed with a rewritten history, and why theit()title shipped here has been left alone on later revisions: either would silently re-parent or conflict the open PRs above.Test: pin the integrations web entry point to the node public surface (stacked on #236) #513 is the exception, and is now superseded. Note the near-miss in its branch name,
fix/wire-unit-integrations-vitest-project— one verb from this branch'sfix/run-unit-integrations-vitest-project, the classic signature of two agents solving the same ticket. It was checked by diff rather than by title (gh pr diff 513 --name-only→integrations/test/index_web_test.ts, a single new file). Since Output 3 now ships here at review request, Test: pin the integrations web entry point to the node public surface (stacked on #236) #513's diff against its own base is empty; a comment on it says so, and closing it is its author's call, not something done unilaterally from here.Fix: raise vitest coverage thresholds to the measured worst-leg floor #324 (
vitest.config.tscoverage thresholds → measured worst-leg floor) is the ratchet that the coverage note below defers. It has since been filed and is disjoint from this diff.Test: fail CI when a root test script targets a vitest project no workflow runs #418 (fail CI when a root test script targets a vitest project no workflow runs) is the mirror image of Test: fail CI when a vitest project is not run by any npm script #343 and is checked here because it is the closest thing to a competitor: it guards the script → workflow direction, while Test: fail CI when a vitest project is not run by any npm script #343 guards the project → script direction that this PR fixes by instance. It touches only
tests/integration/repo_config/workflow_scripts_test.tsand changes no script, so it neither collides with nor supersedes this change.Feat: port FirestoreSessionService from adk-python to @google/adk-integrations #466 (
FirestoreSessionService→@google/adk-integrations) is the one genuine line-level overlap, found by diffing rather than by title: it editsintegrations/test/version_test.tstoo, bumping the stale literal'1.3.0'→'1.5.0'as a side-change inside a feature PR. It is not a competing fix so much as a demonstration of the problem — a literal re-rots at the next release, which is how the assertion got to be two releases stale in the first place, and Feat: port FirestoreSessionService from adk-python to @google/adk-integrations #466 does not wire the project into any script, so the two new test files it adds (integrations/test/firestore/fake_firestore_test.ts,integrations/test/firestore/firestore_session_service_test.ts) would land just as orphaned as the one already there. This PR is the older of the two (2026-07-29 vs 2026-08-01) and is the dedicated fix, so it was not re-parented onto a feature branch that depends on it rather than the other way round. Whichever merges second resolves a small conflict on that one file; keeping the derived assertion is the correct resolution in either order, and it then also covers Feat: port FirestoreSessionService from adk-python to @google/adk-integrations #466's bump for free.vitest.config.tsis touched by five other open PRs (Chore: hoist duplicated vitest project alias map into a shared constant #261, Fix: raise vitest coverage thresholds to the measured worst-leg floor #324, Chore: derive the vitest coverage include list from the npm workspaces #362, Feat: warn in CI when the vitest coverage thresholds fall behind real coverage #377, Chore(test): hoist the shared vitest alias map into one constant #434). This PR no longer touches that file at all, so it cannot conflict with any of them — one more reason the comment described above was dropped rather than shrunk further.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.
[x] All unit tests pass locally.
1. The bug, reproduced before the fix. With the original test restored, the project that CI never ran is red:
2. After the fix:
3. Mutation proof — the new test can still fail. Coverage is not proof, so the assertion was mutated in both directions, one file at a time:
integrations/src/version.ts→export const version = '9.9.9';AssertionError: expected '9.9.9' to be '1.4.0'integrations/package.jsonversion→0.0.1(source left at1.4.0)AssertionError: expected '1.4.0' to be '0.0.1'Mutating each side one file at a time is the point: the second row is what proves the expected value is genuinely read from
integrations/package.jsonand is not accidentally derived from the same module as the actual value — a same-source comparison would pass both mutations. Both were reverted (git checkout) and the working tree confirmed clean withgit status --porcelain.3b. Mutation proof for
index_web_test.ts, with each mutation chosen so that exactly one of its two assertions fires. That separation is the evidence that the secondit()is not redundant with the first — two assertions can both be "covered" while their failure modes overlap completely, and this shows they do not:export const dummy = 1;appended tosrc/index.tsonly (export added to one entry point)AssertionError: expected [ 'version' ] to deeply equal [ 'dummy', 'version' ]; other 2 passsrc/index_web.tsre-export replaced withexport const version = '0.0.0-divergent';expected '0.0.0-divergent' to be '1.4.0'The second row is the interesting one: the two entry points still agree on export names and disagree on the value behind them, so only a test that compares bindings rather than key sets catches it. Both mutations were reverted with
git checkoutandgit status --porcelainconfirmed empty.4. Durability proof — the reason for deriving instead of hardcoding. Simulating a release-please bump by setting both
integrations/package.jsonandintegrations/src/version.tsto the same next version, exactly the pair of edits a release commit makes:It still passes with no edit to the test — exactly what a hardcoded literal could not do, and the reason this project going stale a second time is now structurally prevented. Both files were reverted afterwards.
This was re-run against the version pair
mainactually carries today (1.5.0in bothintegrations/src/version.tsandintegrations/package.json), not just a hypothetical one, since this branch is based on the1.4.0state and merges forward into1.5.0: with both files at1.5.0the test passes1 passed (1), and half-bumping onlyintegrations/package.jsonto1.6.0still FAILS withexpected '1.5.0' to be '1.6.0'. So the assertion survives the merge without an edit and keeps its teeth after it. (git merge-tree --write-tree fork/main HEADalso exits 0 — the merge itself is clean; see the note on stacked PRs below for why the branch is not rebased.)5. Project selection, with a negative control. All three unit selectors resolve (vitest errors on an unknown project name):
The negative control is what actually pins the wiring, so it was run explicitly rather than inferred from file counts. With the
'9.9.9'mutation from proof 3 still in place, the old two-project selector was re-run:The same injected bug is invisible under the old script and fatal under the new one. That difference is the entire content of this PR: the test file is not merely unasserted-against, it is never even collected.
grep -n "unit:integrations" package.jsonnow returns the three script lines (27, 28, 32) instead of nothing.All proofs above were re-run after
vitest.config.tswas restored to its unmodified state, since a config file is exactly where a "no longer touched" claim is worth checking rather than asserting:git diffon that file is empty, the project still resolves and the suite still passes (Test Files 2 passed (2) / Tests 3 passed (3)), and every mutation still fails with the messages in the tables above.6. Static gates.
ts:checkfails identically on unmodifiedmain(308 pre-existing errors in thecore/dev/teststrees, the subject of separate PRs) and is not part ofvalidation.yaml. The error sets onmainand on this branch are byte-identical — verified bydiff-ing the two logs, not by comparing counts — so this change introduces zero new type errors, which also confirms the JSON import attribute typechecks undernodenext. Nothing underintegrations/is flagged.Both integrations test files are genuinely in that typecheck program rather than silently excluded from it, which is the part worth checking before quoting a zero:
npx prettier --checkandnpx eslintwere also run againstintegrations/test/index_web_test.tsspecifically; both exit 0. The diff adds noany, noascast, and no@ts-expect-error,@ts-ignore,eslint-disableor coverage-tool suppression of any kind.One local-only caveat worth recording, since it is easy to misattribute to this PR: running
npm run test:unitin a shell that exportsGOOGLE_CLOUD_PROJECT/GOOGLE_CLOUD_LOCATIONfailsdev/test/cli/cli_create_test.ts > should handle Vertex AI selection with gcloud defaults. That is ambient-environment leakage in an untouchedunit:devtest, not a regression here —env -u GOOGLE_CLOUD_PROJECT -u GOOGLE_CLOUD_LOCATION npx vitest run --project unit:dev dev/test/cli/cli_create_test.tspasses 10/10, CI (which has no such vars) is green, and PRs #203 and #259 already target that hermeticity gap.7. Coverage.
npm run test:coverage— the exact commandvalidation.yaml:41runs — cannot complete in this sandbox: 22 tests fail withAPI key must be provided via constructor or GOOGLE_GENAI_API_KEY or GEMINI_API_KEY environment variableand 4 more withCommand failed: npx @google/adk-devtools --version(no npm registry egress). Per the rule for failures outsideintegrations/test, this was verified against unmodifiedmainrather than worked around: the same command onmainfails too, and the set of failing test files on this branch is a strict subset ofmain's (mainadditionally fails the knownapp_loader_test.tsflake). No failure is unique to this branch and none is inintegrations/. The run also confirms the project is now picked up: 235 test files collected here versus 234 onmain.Because the local run aborts, v8 never prints the summary table there — so the real numbers below are taken from CI, read out of the
All filesrow of the three OS legs of the run on this PR's current head commit:Every metric moved up relative to the revision without Output 3 (ubuntu:
90.68 / 89.61 / 91.55 / 90.68→90.69 / 89.62 / 91.63 / 90.69), which is the aggregate footprint of takingindex_web.tsfrom 0% to 100%. Small, because the integrations package is ~20 lines against the whole ofcore/src+dev/src— and monotonically upward, which is the property that matters: no threshold can regress.integrations/srcis now at 100% on every metric. All three files were at 0% before this PR, because nothing executed them; measured directly against this branch's head:An earlier revision of this PR left
index_web.tsat0 / 0 / 0 / 0(lines 1-7 uncovered) — that is the gap Output 3 closes.The coverage thresholds in
vitest.config.tsare left where they are, including their dated comment, even though every measured floor now exceeds them. (This PR makes no edit to that file at all — no threshold, glob, comment or project definition moves.) Raising them here would be unrelated churn: the gap is ~6 months of drift since the 2026-02-06 baseline, not an effect of this PR, which contributes roughly two statements against the whole ofcore/src+dev/src. It would also be risky to do casually — the three legs disagree (branches span 89.45–89.65 because they skip different suites), so a threshold set from the best leg red-lines every open PR on the weakest one. The ratchet is worth doing on its own, with its own verification, and has been filed as a separate task rather than smuggled in here. Nothing in this change lowers coverage, so leaving the gate as-is regresses nothing.Manual End-to-End (E2E) Tests:
No credentials or configuration are required.
Step 2 is the observable behaviour change: those test names appear in no CI log anywhere before this PR.
CI result. Green on all legs —
run-tests (ubuntu-latest),run-tests (macos-latest),run-tests (windows-latest), pluscheck-license. These are the real build/test jobs, not just the trivial checks.Green CI only proves nothing broke; it does not by itself prove the newly wired project ran. That was read back out of the workflow log on all three legs rather than inferred from the summary:
Those log lines are the deliverable: it is the first time either test has appeared in a CI log at all. The Windows leg in particular confirms both the JSON module import and the relative
../src/*.jsspecifiers need no path handling to be portable.Three infrastructure flakes were hit along the way and are recorded rather than hidden, since all are in suites this diff does not touch:
Windows, on the commit that added Output 3 (
940babf5b):core/test/code_executors/unsafe_local_code_executor_test.ts > UnsafeLocalCodeExecutor > should execute shell code and return stdout→Test timed out in 5000ms. Not this diff — it is aunit:coretest onmainthat shells out toecho "Hello, Shell!"through a real child process with no explicitit()timeout, so it runs against the 5s default, and process spawn on a loaded Windows runner does not reliably fit in 5s. Attributed by evidence rather than assertion: theunit:integrationsproject passed on that same red leg (✓ index_web_test.ts (2 tests) 9ms,✓ version_test.ts (1 test) 6ms), the failure is in a file this branch does not modify (confirmed withgit diff main --name-only), and the reported location:161matches the file on the base the PR merges into, not this branch. Green on rerun of the identical commit, which is what makes it a flake rather than a diagnosis. Worth a separate timeout fix, but not one to smuggle into this PR.macOS, twice:
tests/integration/app_loader/app_loader_test.ts > should discover apps vs agents across directories and standalone files→Test timed out in 40000ms(1 failed / 224 passed). Classified by evidence, not assumption: the branchfix/app-test-dummy-agent-abstract-members, whose diff is unrelated and which is based on a newermainthan this one, failed the macOS leg on the identical test and timeout three hours earlier. The timeout is the test's own explicitTEST_EXECUTION_TIMEOUT = 40000third argument toit(), so it is not affected by any project-leveltestTimeout. Green on rerun.Windows, earlier:
Error: CLI exited prematurely with code 1fromtests/integration/a2a/input_required/input_required_test.tsattests/integration/test_case_utils.ts:341— a child-process spawn flake (that run reported 2510 tests passed and 0 test-level failures; what failed was the suite's server process, not an assertion). Green on rerun. In the latest run Windows was not a failure at all: the matrix cancelled it when macOS went red.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.