Skip to content

Test: pin the dev server eval-set read API at the HTTP level - #208

Open
AmaadMartin wants to merge 1 commit into
mainfrom
feat/dev-server-eval-set-read-api-tests
Open

Test: pin the dev server eval-set read API at the HTTP level#208
AmaadMartin wants to merge 1 commit into
mainfrom
feat/dev-server-eval-set-read-api-tests

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 10, 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: The developer UI browses eval data through six routes on DevServer: list eval sets (canonical and deprecated), list the eval case ids in a set, and get, update or delete a single eval case. No test drives those routes over HTTP. A route rename, a response-shape change, or a status-code drift ships silently.

Solution: I added 13 tests to the existing # Eval endpoint plumbing section of tests/unittests/cli/test_fast_api.py. They drive the real FastAPI app with TestClient over a real InMemoryEvalSetsManager, so nothing on the path is mocked. Every error path asserts the exact status code and the exact detail string. No file under src/ changes.

Two handlers behave in a way that looks like a defect. list_evals_in_eval_set answers 400 for a missing eval set while its get, put and delete siblings answer 404. list_eval_sets returns an empty list for an unknown app instead of 404. The tests pin today's behaviour and carry an inline comment, because changing a public status code is a separate change.

Collision check: I listed the 100 open pull requests on this fork and read the diff of every one that touches tests/unittests/cli/test_fast_api.py or src/google/adk/cli/dev_server.py. No open pull request adds HTTP tests for these routes. Three overlap:

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:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
pytest tests/unittests/cli/test_fast_api.py -q
122 passed, 5 skipped, 2 xfailed in 30.79s

The 13 tests expand to 17 items, because four are parametrized over two cases. The file held 112 items before and holds 129 now. The existing pass, skip and xfail counts do not change, and no new warning appears.

Mutation results. The dev extra installs no coverage plugin, so I proved the tests instead of measuring them. I broke one production line per run and confirmed the matching test fails. All 11 mutations were killed, and together they cover all 17 new items.

Mutation Test that failed Failure
drop sorted() in list_evals_in_eval_set ..._returns_sorted_eval_case_ids assert ['case_b', 'case_a'] == ['case_a', 'case_b']
list_evals_in_eval_set raises 404, not 400 ..._unknown_eval_set_is_a_client_error assert 404 == 400
list_eval_sets reverses the ids ..._returns_created_eval_set_ids assert {'evalSetIds': ['es_two', 'es_one']} == {'evalSetIds': ['es_one', 'es_two']}
list_eval_sets raises 404 on an empty result ..._is_empty_for_app_without_eval_sets assert 404 == 200
legacy list route returns the envelope ..._legacy_returns_a_bare_id_list ResponseValidationError: Input should be a valid string
get_eval never returns the found case test_get_eval_case_returns_the_eval_case (2 items) assert 404 == 200
get_eval reports a generic detail ..._returns_404_for_missing_eval_set_or_eval_case (2 items) assert 'not found.' == 'Eval set `es_one` or Eval `missing` not found.'
update_eval skips the id mismatch check ..._rejects_eval_id_mismatch assert 200 == 400
update_eval never writes to the manager ..._replaces_the_stored_eval_case, ..._returns_404_... (3 items) assert 'user text for case_a' == 'replacement text'
delete_eval never deletes ..._removes_it_from_the_eval_set, ..._returns_404_... (3 items) assert ['case_a', 'case_b'] == ['case_a']
EvalCase drops the conversation xor scenario check ..._rejects_a_body_without_a_conversation assert 200 == 422

Manual End-to-End (E2E) Tests:
TestClient already exercises the real routing table, the real pydantic serialization and the real InMemoryEvalSetsManager, so the automated tests are the end-to-end proof. To reproduce:

uv sync --extra test --extra dev
pytest tests/unittests/cli/test_fast_api.py -q -k "eval"

I recorded the expected bodies by driving the real app first, then wrote the assertions from what it returned.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

CI note

Every test job passes: Unit Tests on Python 3.10 through 3.14, A2A v0.3 Tests on 3.10 through 3.14, and Mypy Check on 3.10 through 3.13.

Pre-commit Linter fails, and it fails for a reason this branch did not cause. The update-constraints hook rewrites constraints-3.11.txt and constraints-3.12.txt during the run, so pre-commit reports "files were modified by this hook". The same job fails on main itself at commit 352d11d, and on the other open pull requests. This branch touches no pyproject.toml and no constraints file. Running pre-commit against the one file I changed passes every hook:

pre-commit run --files tests/unittests/cli/test_fast_api.py
isort ... Passed   pyink ... Passed   addlicense ... Passed
ADK Compliance Checks ... Passed   codespell ... Passed

The dev UI browses eval data through six routes on DevServer, and none of
them had an HTTP-level test. A rename, a response-shape change, or a
status-code drift could ship unnoticed.

These tests drive the real FastAPI app with TestClient over a real
InMemoryEvalSetsManager, and assert the exact status code and detail
string on every error path.
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