Skip to content

Test: alias @google/adk-devtools to source in the unit:core vitest project (stacked on #374) - #542

Closed
AmaadMartin wants to merge 0 commit into
feat/export-version-from-adk-devtoolsfrom
feat/export-version-from-devtools
Closed

Test: alias @google/adk-devtools to source in the unit:core vitest project (stacked on #374)#542
AmaadMartin wants to merge 0 commit into
feat/export-version-from-adk-devtoolsfrom
feat/export-version-from-devtools

Conversation

@AmaadMartin

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A

  2. Or, if no issue exists, describe the change:

Problem: The unit:dev vitest project resolves @google/adk-devtools to dev/src, so its tests read TypeScript source. unit:core was left resolving the package through the npm-workspace symlink to dev/dist, i.e. build output. A core/test file importing the package would therefore silently exercise a stale (or, before a first npm run build, missing) bundle rather than source.

Solution: Declare the same '@google/adk-devtools' -> dev/src alias in the unit:core project. This mirrors how @google/adk-integrations is already declared in unit:core without a current importer, so the alias map states one consistent rule for the unit projects: workspace packages resolve to source.

integration, e2e, cross-language and unit:integrations are deliberately not touched. tests/integration/adk_web/webui_test.ts imports AdkApiServer on purpose to exercise the built package: dev/src/server/adk_api_server.ts serves the debug UI from path.join(__dirname, '../../browser'), which only resolves when running from dev/dist/esm/server/ (__dirname there is injected by the esbuild shim, not declared in source). Aliasing that project to dev/src would point it at a nonexistent dev/browser and break that test's statusCode === 200 / <app-root> assertions.

I also corrected the adjacent comment on unit:dev, which claimed the alias was declared "in this project only" — my line makes that statement false.

Collision check (required, and load-bearing here).

gh pr list --repo AmaadMartin/adk-js --state open --limit 1000

441 open PRs; I inspected every plausibly adjacent one with gh pr diff --name-only:

Scope note. The residual delta over #374 is deliberately just the unit:core alias. unit:core includes only core/test/**/*_test.ts, and nothing there imports devtools today (grep -rn "adk-devtools" core/test/ returns 0), so this line is inert until someone writes such a test. I have not manufactured a core/test -> devtools import to fabricate a consumer for it: core must not depend on dev, and adding that import purely to cover a config line would be a worse precedent than the inert line. Flagging this explicitly rather than leaving a reviewer to discover it.

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.

No new test file. The behaviour this PR adds is test-time module resolution in a project that currently has no importer; the only test that could cover it would be a core/test file importing @google/adk-devtools, which is the inverted dependency described above. Instead I verified the line empirically and by mutation, and no existing test was modified, skipped, or deleted.

Mutation testing — every claim below was run, not assumed. dev/dist was made stale (the version export stripped from dev/dist/esm/index.js) to distinguish "resolved from source" from "resolved from build output".

# Mutation Result
A Remove export {version} from './version.js'; (from the base branch) FAILAssertionError: expected undefined to be '1.5.0', plus TypeError: .toMatch() expects to receive a string, but got undefined
B0 Remove the unit:dev alias, dev/dist fresh PASS — see caveat below
B1 Remove the unit:dev alias, dev/dist stale FAILAssertionError: expected undefined to be '1.5.0'
B2 Restore the unit:dev alias, dev/dist still stale PASS — proves the alias makes the test read dev/src
C1 My line present, dev/dist stale, throwaway probe in core/test importing @google/adk-devtools PASS — unit:core resolves to dev/src
C2 My line removed, same probe FAILAssertionError: expected undefined to be defined

Caveat worth recording (B0): immediately after npm run build, the base branch's dev/test/index_test.ts still passes with the unit:dev alias removed, because the freshly built dev/dist contains the export. That test therefore pins the export, not the alias. B1/B2 isolate the alias's actual contribution — immunity to stale or absent build output — which is the same property this PR extends to unit:core. The C1/C2 probe was a scratch file used only for this proof and is not committed.

Commands run on the exact pushed commit:

npm run build                                             # required: tests/global_setup.ts imports @google/adk via core/dist
npx vitest run --project unit:dev dev/test/index_test.ts  # 2 passed
npx vitest run --project unit:dev                         # 224 passed, 1 pre-existing failure (see below)
npx vitest run --project unit:core core/test/utils core/test/models/base_llm_test.ts
                                                          # 21 files, 277 passed
npm run lint                                              # clean
npm run format:check                                      # clean
bash scripts/check_license.sh                             # clean
npm run ts:check                                          # 280 errors, identical with and without this change

Two pre-existing conditions, both confirmed unrelated by stashing this change and re-running:

  • dev/test/cli/cli_create_test.ts > "should handle Vertex AI selection with gcloud defaults" fails identically on the base branch. It is an ambient-environment leak: the test expects a mocked gcloud default but receives the developer machine's real GCP project. Fork PR Fix: scrub ADK environment variables from unit test runs #302 targets this hermeticity class.
  • npm run ts:check reports 280 errors on the base branch (mostly BASE_AGENT_SIGNATURE_SYMBOL assignability in core/test/a2a/*). This change adds zero — the count is byte-identical with and without it.

Manual End-to-End (E2E) Tests:

# 1. Prove unit:dev reads source, not build output:
npm install && npm run build
perl -ni -e 'print unless m{version}' dev/dist/esm/index.js   # make dist stale
npx vitest run --project unit:dev dev/test/index_test.ts      # still passes -> read from dev/src
npm run build                                                 # restore dist

# 2. Prove this PR's unit:core alias resolves (scratch file, delete afterwards):
cat > core/test/probe_test.ts <<'EOF'
import {version} from '@google/adk-devtools';
import {describe, expect, it} from 'vitest';
describe('probe', () => {
  it('resolves', () => { expect(version).toBeDefined(); });
});
EOF
npx vitest run --project unit:core core/test/probe_test.ts    # passes with this PR
rm core/test/probe_test.ts

Note for anyone reproducing: npx vitest fails with Failed to resolve entry for package "@google/adk" unless npm run build has run first, because tests/global_setup.ts imports @google/adk, which resolves through core/dist rather than the project alias.

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.

@AmaadMartin

Copy link
Copy Markdown
Owner Author

Closing this PR: after complexity review it has no content left.

This branch was stacked on #374 and carried exactly one authored commit — adding the @google/adk-devtools alias to the unit:core vitest project. The complexity review flagged that hunk as YAGNI, and the objection holds on inspection:

  • Nothing under core/test/ or core/src/ imports @google/adk-devtools, so the alias cannot affect resolution for any test that exists.
  • My justification was parity with the neighbouring @google/adk-integrations entry — but that specifier has zero importers anywhere outside integrations/ (the only hits in the repo are the six alias declarations in vitest.config.ts itself). Copying dead config by analogy to dead config is not a reason.

Dropping the commit also removes the need for the follow-on comment fix: #374's "aliased to source in this project only" is accurate again once unit:core has no alias.

Everything of substance in the originating task — the version re-export from @google/adk-devtools and the unit:dev source alias — is already delivered by #374, which remains open. No competing implementation was created. Review #374 instead.

@AmaadMartin AmaadMartin closed this Aug 2, 2026
@AmaadMartin
AmaadMartin force-pushed the feat/export-version-from-devtools branch from a3fc16d to e21127e Compare August 2, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant