Test: fail integration tests fast when the ADK CLI is not built - #673
Open
AmaadMartin wants to merge 1 commit into
Open
Test: fail integration tests fast when the ADK CLI is not built#673AmaadMartin wants to merge 1 commit into
AmaadMartin wants to merge 1 commit into
Conversation
AdkTsApiServer.start() spawned node on dev/dist/esm/cli_entrypoint.js without checking that the build output exists. On a fresh checkout the child exits 1 and the harness reports only "CLI exited prematurely with code 1", which names neither the missing file nor the fix. assertAdkCliBuilt() now runs as the first statement of start() and names the resolved path and the npm run build command. This mirrors the existing prerequisite check in AdkGoServer.start().
7 tasks
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
Link to an existing issue (if applicable):
Or, if no issue exists, describe the change:
Problem:
AdkTsApiServer.start()spawnsnode dev/dist/esm/cli_entrypoint.jswithout checking that the file exists. A contributor who runs an integration test beforenpm run buildgets a rawERR_MODULE_NOT_FOUNDdump and thenCLI exited prematurely with code 1. That message names neither the missing file nor the fix. Five suites reach this path, includingtests/integration/a2a/basic/a2a_agent_test.tsandtests/integration/adk_web/webui_test.ts.Solution:
assertAdkCliBuilt()runs as the first statement ofstart(). It names the resolved path and tells the developer to runnpm run build, which is the orderCONTRIBUTING.mddocuments.AdkGoServer.start()already checks a prerequisite withfs.existsSyncin that position, so this follows an in-repo precedent. The guard is exported at module scope so a test can call it without widening the privategetAdkCliArgs.Scope notes:
tests/integration/test_api_server.tschanges. The timeout path (an entrypoint that exists but never prints its banner) is out of scope and still costs the full 60 s.webui_test.tsand also coversdev/dist/browser/index.html, which only that suite needs. Fix: allocate integration test-server ports from the OS and attach child output to start-up failures #546 touches this file, but it only removes asuccessLogMessageline. No open pull request adds this guard, so this branch is based onmain, not stacked.Testing Plan
Unit Tests:
New file
tests/integration/test_api_server_test.ts, 3 tests:Proof the tests can fail. I mutated the source twice and re-ran the same file:
assertAdkCliBuilt(CLI_ENTRYPOINT_PATH);fromstart()AssertionError: promise resolved "AdkApiClient{ backendUrl: 'http://lo…' }" instead of rejectingif (fs.existsSync(cliPath))AssertionError: expected [Function] to throw an error, andexpected [Function] to not throw an error but 'Error: ADK CLI entrypoint not found a…' was thrownCoverage:
vitest.config.tsmeasurescore/src,dev/srcandintegrations/srconly, so files undertests/are not reported. Both branches of the newifexecute, and thestart()call site is pinned by the third test.Manual End-to-End (E2E) Tests:
Reproduce an unbuilt entrypoint on a built tree, then restore it:
Before this change the run ended in
CLI exited prematurely with code 1. Now it fails in 4 ms of test time with:Behaviour on a built tree is unchanged. Both dependent suites still pass:
Checklist