Skip to content

Test: build the tool_auth_handler suite from real Contexts - #845

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/tool-auth-handler-test-real-context
Open

Test: build the tool_auth_handler suite from real Contexts#845
AmaadMartin wants to merge 2 commits into
mainfrom
fix/tool-auth-handler-test-real-context

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):
    N/A
  2. Or, if no issue exists, describe the change:
    Problem: tool_auth_handler_test.ts forced object literals into the Context type with as unknown as 11 times. Each stub replaced getAuthResponse, requestCredential and the delta-aware State with vi.fn(), so the suite could not see the real key derivation or the real state plumbing. A drift in either one kept all nine tests green.

Solution: Every tool context is now a real Context over a real Session, built by one local factory. The credential exchanger is a single hoisted mock, typed from BaseCredentialExchanger['exchange'] so it cannot drift from the interface it stands in for, shared by each constructed instance; that removes the last cast. The nine test titles and every assertion are unchanged apart from their receiver, plus two assertions that pin the effects the real methods now produce.

Two deliberate deviations from the plan, both grounded in this file:

  • The six incomplete {type: 'apiKey'} literals become one API_KEY_SCHEME constant instead of six repeated literals. Its name is X-API-Key, the value the file already used at its two complete literals.
  • Mutation 5 (an own-property write in place of State.set) was predicted to be invisible to the old stubs. I measured it: the old fixtures also fail it, 3 tests. Mutation 9 below is the real gap.

Collision check. gh pr list --repo AmaadMartin/adk-js --state open --limit 300, then gh pr diff --name-only on every plausibly adjacent PR. Four open PRs touch this file and none of them does this change:

PR What it does here Overlap
#742 Adds name/in to the six apiKey literals, keeps the casts This PR supersedes that hunk
#764 Extracts an API_KEY_SCHEME constant, keeps the casts Same constant, same value; this PR supersedes it
#772 Adds a new describe block at the end (plus a src change) Appends after my last test
#774 Adds two new tests at the end (plus a src change) Appends after my last test

This PR is based on main, as the approved plan directs. Stacking is not possible on four independent bases. On a conflict with #742 or #764, take this branch's version of the file; #772 and #774 append and should merge cleanly.

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.

Commands, run at d75eeaa4:

$ npx vitest run --project unit:core core/test/tools/openapi_tool/tool_auth_handler_test.ts
 Test Files  1 passed (1)
      Tests  9 passed (9)

$ npx tsc --noEmit 2>&1 | grep 'core/test/tools/openapi_tool/tool_auth_handler_test.ts'
before: 6 errors (TS2345, the incomplete {type: 'apiKey'} literals)
after:  0 errors

$ npx eslint core/test/tools/openapi_tool/tool_auth_handler_test.ts     # clean
$ npx prettier --check core/test/tools/openapi_tool/tool_auth_handler_test.ts
All matched files use Prettier code style!

$ grep -c 'as unknown as' core/test/tools/openapi_tool/tool_auth_handler_test.ts
before: 11    after: 0

$ npm run build     # ok
$ npm run lint      # ok

No product code changed, so coverage is unchanged by construction.

Mutation proof. Each mutation is applied to core/src/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.ts, then reverted.

# Mutation Tests that fail Message
1 Delete the if (!this.authScheme) guard should return done if no auth scheme Cannot read properties of undefined (reading 'type')
2 const authResponseCredential = undefined; should return done after exchange..., should store exchanged credential..., re-uses a credential persisted... expected 'pending' to be 'done'
3 Delete the requestCredential(authConfig) call should return pending and request credential... expected "requestCredential" to be called at least once
4 Delete the if (existingCredential) early return should return cached credential if available, re-uses a credential persisted... expected 'pending' to be 'done'
5 Own-property write in place of state.set(key, credential) should store exchanged credential..., re-uses a credential persisted..., caches a static credential... expected undefined to be 'exchanged-token'
6 Drop the ?? this.authCredential fallback uses the credential the tool was configured with..., does not copy a static credential..., caches a static credential... expected 'pending' to be 'done'
7 Cache condition to if (true) does not copy a static credential... expected { authType: 'apiKey', …(1) } to be undefined
8 Cache condition to if (authResponseCredential) caches a static credential... expected undefined to be 'exchanged-token'

Each of the nine tests is killed by at least one mutation.

Two more mutations show what the stubs hid. Both were run against the old fixtures and the new ones.

# Mutation Old fixtures New fixtures
9 auth_handler.ts: auth-response key prefix temp: to temp_ 9 passed 3 failed
10 context.ts: requestCredential stops writing to eventActions.requestedAuthConfigs 9 passed 1 failed, expected undefined to be defined

Manual End-to-End (E2E) Tests:
No E2E test. This is a test-fixture change with no product code in the diff, so there is no runtime behaviour to exercise. To check it by hand:

npm ci
npm run build --workspace=core
npx vitest run --project unit:core core/test/tools/openapi_tool/tool_auth_handler_test.ts

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.

Amaad Martin added 2 commits August 8, 2026 22:13
The suite forced object literals into the Context type with 'as unknown as',
so it stubbed getAuthResponse, requestCredential and State. A change to the
auth-response key derivation or to the State delta wiring could not fail a
test.

Every tool context is now a real Context over a real Session, the credential
exchanger is one hoisted mock shared by each constructed instance, and the six
incomplete apiKey scheme literals become one typed constant. All nine test
titles and every assertion are unchanged apart from their receiver.
The hand-copied parameter and return types could drift from the interface
they mock. Deriving the mock from BaseCredentialExchanger['exchange'] keeps
them in step and drops two imports that only spelled the signature out.
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