Skip to content

Test: cover module type resolution by file extension and the --file_type override - #874

Open
AmaadMartin wants to merge 1 commit into
mainfrom
fix/agent-loader-module-type-extension-tests
Open

Test: cover module type resolution by file extension and the --file_type override#874
AmaadMartin wants to merge 1 commit into
mainfrom
fix/agent-loader-module-type-extension-tests

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 9, 2026

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):

  2. Or, if no issue exists, describe the change:
    Problem: getFileModuleType() picks a module type from the file extension before it reads package.json. Four of its five outcomes have no test: .cjs/.cts, .mjs/.mts, the unknown-extension default, and the AgentFileOptions.moduleType override behind the --file_type flag. A refactor can therefore change the compiled file extension and the esbuild format while the suite stays green.

Solution: I added seven tests to dev/test/utils/agent_loader_test.ts. They drive the public AgentFile class and assert the recorded esbuild.build options, because getFileModuleType is module-private and must stay private. This is a test-only change; no production file is modified.

Every compiled fixture uses an ext_ basename prefix. The Vitest module runner caches an imported module by path and ignores the cache-busting query that AgentFile.load() appends, so a repeated basename returns the earlier test's module.

Collision check. I ran this before starting:

gh pr list --repo AmaadMartin/adk-js --state open --limit 100 --json number,title,headRefName
gh pr diff 778 --repo AmaadMartin/adk-js   # package.json resolution, not the extension branches
gh pr diff 870 --repo AmaadMartin/adk-js   # --file_type help text in cli.ts, no test overlap

No open PR covers the extension branches or the moduleType override. PR #778 edits the same test file for the package.json half of the decision. This branch stays based on main so it carries none of that PR's work, and the two diffs sit in different regions of the file.

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.

npm run build -w core
npx vitest run --project unit:dev dev/test/utils/agent_loader_test.ts   # 38 passed (31 existing + 7 new)

Proof the tests can fail. I mutated dev/src/utils/agent_loader.ts, re-ran the file, and reverted each time.

Mutation Result
M1: the ['.mts', '.mjs'] branch returns CJS 2 failed | 36 passed — ext_mjs_agent.mjs, ext_mts_agent.mts
M2: the ['.cjs', '.cts'] branch returns ESM 2 failed | 36 passed — ext_cjs_agent.cjs, ext_cts_agent.cts
M3: the final return FileModuleType.CJS returns ESM 1 failed | 37 passed — ext_unsupported_agent.jsx
M4: drop this.options.moduleType || from AgentFile.load() 2 failed | 36 passed — both prefers an explicit ... moduleType tests

M4 failure message:

AssertionError: expected { …(10) } to match object { …(2) }
-   "format": "esm",
-   "outfile": ".../ext_explicit_esm_agent.mjs",
+   "format": "cjs",
+   "outfile": ".../ext_explicit_esm_agent.cjs",

Known gap, disclosed. Deleting the ['.cjs', '.cts'] branch is not detectable. Those extensions then reach the same return FileModuleType.CJS default, so the result does not change. The tests pin the resolved value, not the existence of the branch.

Other local checks on the pushed commit:

npm run build     # clean
npm run lint      # clean
npx prettier --check dev/test/utils/agent_loader_test.ts   # clean
npx vitest run --project unit:dev   # 1 failed | 251 passed

Two pre-existing problems, unrelated to this change. cli_create_test.ts > should handle Vertex AI selection with gcloud defaults reads the machine's real gcloud config and fails on main too. npx tsc --noEmit reports 287 errors, all in core/test/ and tests/integration/, none in dev/, and the count is identical with and without this change.

CI is green on ubuntu, macOS and Windows. The first Windows run timed out after 40 s in tests/integration/app_loader/app_loader_test.ts, which this change does not touch; it passed on a rerun of the same commit.

Manual End-to-End (E2E) Tests:
None. No behaviour ships. The tests pin what a user already gets from adk run path/to/agent.mjs (ESM, no package.json read) and adk run path/to/agent.js --file_type esm (explicit override).

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.

…type override

getFileModuleType() maps a file extension to a module type before it reads
package.json, and AgentFile.load() lets AgentFileOptions.moduleType override
the whole lookup. Four of the five branches had no test, so a refactor could
change the compiled extension and the esbuild format without a red suite.

Test-only change. No production file is modified.
@AmaadMartin
AmaadMartin force-pushed the fix/agent-loader-module-type-extension-tests branch from 1e50f23 to c4f456d Compare August 9, 2026 16:35
@AmaadMartin
AmaadMartin changed the base branch from fix/agent-loader-test-package-json-module-type to main August 9, 2026 16:35
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