test(agentkit): unit tests for apps entrypoints + deploy-pipeline (surface + core logic)#105
Merged
yaozheng-fang merged 2 commits intoJul 1, 2026
Conversation
Targets the two highest-risk under-tested areas found by a coverage audit: the deployable app server surfaces (apps/, ~9.9% -> 76.3%) and the deploy pipeline logic (toolkit runners/executors/strategies). Overall line coverage 45.9% -> 50.9%. +258 tests, all offline (no network/docker/uvicorn). apps/ (Starlette TestClient + asyncio.run, telemetry singletons spied): - simple_app: health/readiness/liveness + /ping wiring + ping() arg guard - simple_app_handlers: InvokeHandler signature dispatch + response matrix, _convert_to_sse, AsyncTaskHandler, PingHandler - a2a_app / mcp_app: decorator guards, ping endpoint, env-detect routes/tools - agent_server: AgentKitAgentLoader + ctor guards; telemetry latency gating and error_type formatting; ASGI middleware header-exclusion + finish gating pipeline (fakes injected via the existing DI seams): - hybrid_strategy: _should_push_to_cr / _validate_cr_image_url error mapping, build/deploy config-updates assembly, invoke/status/destroy passthrough - base_executor: _classify_error / _handle_exception (pins the ErrorCode mapping introduced by the engineering-standards work), preflight handling - init_executor: render context, template/name validation, project scaffold - runners/base + ve_agentkit: pure payload/detection helpers, _needs_runtime_ update env diffing, deploy guard/dispatch, destroy/status mapping Pins (NOT fixes — this branch is tests-only) three latent source bugs to their current behavior with # NOTE comments, for a follow-up fix: - mcp_app tool()/agent_as_a_tool() wrappers: on a tool exception the finally passes func_result=result while result is unbound -> UnboundLocalError masks the real error and skips telemetry (all 4 wrapper branches). - simple_app_handlers PingHandler._format_ping_status: self.func.__name__ raises AttributeError when no ping func is registered. - simple_app_handlers AsyncTaskHandler.handle: signature omits request, violating the BaseHandler.handle(request) contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st surface)
The prior batch raised the coverage number but left the product's core
orchestration ~0%. This batch tests that spine directly with proper fakes,
asserting behavior (error codes, response shapes, collaborator calls),
lifting the critical functions from ~0% to tested:
ve_pipeline.build (build->push->deploy spine) 1.9% -> 100%
ve_pipeline._upload_to_tos (bucket-ownership gate) 0% -> 70%
ve_agentkit._create_new_runtime 3% -> 91%
ve_agentkit._update_existing_runtime 1.8% -> 88%
ve_agentkit._wait_for_runtime_status 9.5% -> 100%
runners/base._http_post_invoke (invoke transport) 1% -> 91%
builders/local_docker.build 0.7% -> 86%
agent_server_app._invoke_compat (/invoke) 0% -> 89%
Overall coverage 45.9% -> 53.7%. +81 tests, all offline (no network/docker/
uvicorn; polling loops patched). No source modified.
Pins three more latent source bugs to current behavior (# NOTE, for follow-up):
- ve_pipeline._upload_to_tos: the documented ValueError for an unrendered
{{template}} bucket name is caught by the broad `except Exception` tail and
re-raised as a generic Exception, so callers can never catch ValueError.
- ve_agentkit._update_existing_runtime: client_token=generate_client_token()
is passed to UpdateRuntimeRequest, which has no such field and extra="ignore",
so the idempotency token is silently dropped (Create keeps it, Update sends none).
- builders/local_docker.build: BuildResult.build_logs is typed List[str] but
the raw str from DockerManager.build_image is assigned directly on both the
success and failure paths.
Also hardens test_ve_pipeline_upload_tos's AccountDisable assertion to be
cloud-provider-independent (console host varies volcengine vs byteplus and
provider global state leaks across the full suite).
Remaining gap: _execute_build's poll loop is still thin (mocked out to isolate
the build spine) — a follow-up target.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fedc9c8 to
ad13eaf
Compare
d7864d6 to
48e00c6
Compare
yaozheng-fang
approved these changes
Jul 1, 2026
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.
What
Adds unit tests for the two highest-risk under-tested areas found by a coverage audit: the deployable app server surfaces and the deploy-pipeline / invoke core logic. +339 tests, all offline (no network / docker / uvicorn), 0 source modified.
Overall line coverage 45.9% → 53.7%;
apps9.9% → 76%. Critical functions lifted from ~0% to tested:ve_pipeline.build(build→push→deploy spine)_upload_to_tos(bucket-ownership security gate)_create_new_runtime/_update_existing_runtime_wait_for_runtime_statusrunners/base._http_post_invoke(invoke transport)local_docker.buildagent_server_app._invoke_compat(/invoke serving)Latent bugs surfaced
Testing the real logic (not padding coverage) surfaced 6 latent source bugs; they are pinned to current behavior here with
# NOTEcomments and fixed in the stacked fix PR (fix/agentkit-latent-bugs).Notes
TestClient+asyncio.run(no@pytest.mark.asyncio, since pytest-asyncio is undeclared).test_cli_add_harness(an env-leak flake onmain); no new failures.Stack
Base =
feat/agentkit-engineering-standards. Merge that first.🤖 Generated with Claude Code