[Doc] Raise the bar for agent-written tests (#62601)

Signed-off-by: aoshen02 <281812371+aoshen02@users.noreply.github.com>
Co-authored-by: aoshen02 <281812371+aoshen02@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-10-21 10:02:57 +00:00
1 parent e9cc6244a0
commit 9db0da83f5
3 files changed
+41 -1

No files matched your search

+2
View File
@@ -104,6 +104,8 @@ When adding or modifying tests, consider both the coverage and the effectiveness
Assess whether the included test cases are meaningful, concise, and maintainable. Testing should not be exhaustive: local validation is typically sufficient to ensure a baseline correctness of the change. Contributed tests should focus exclusively on areas that are deemed prone to errors, regressions, or complex interactions within the codebase. Recommend deletion of redundant, trivial, or otherwise unnecessary tests: this is one of the most common complaints raised during code reviews.
Check added and changed tests against [`tests/AGENTS.md`](../../../tests/AGENTS.md). For reproducible bugs, the PR must show the new test failing without the fix and passing with it.
Verify that testing actually exercises the new path, rather than passing through an unchanged path or fallback. Ensure added speculative decoding correctness tests assert nonzero acceptance rate, selection tests identify the desired backend, etc.
Aim to write tests that are meaningful, targeted, and maintainable. Avoid redundant or trivial tests that do not contribute to the overall reliability of the test suite. Avoid using mock objects, especially when highly coupled with internal logic.
+3 -1
View File
@@ -88,7 +88,9 @@ When adding tests:
- **Design before you write.** Answer four questions first: what is the module
for, what is its I/O contract, what failure am I guarding against, and what is
the cheapest level that catches it (unit over integration over e2e)?
the cheapest level that actually reproduces it? Fixes to user-visible
behavior need a test through a real entry point (`LLM`, OpenAI server); see
[`tests/AGENTS.md`](tests/AGENTS.md).
- **Reuse before create.** Extend existing test files, `conftest.py` fixtures, and
helpers; add a new file only when no nearby suite fits.
- **Test behavior with intent.** Assert observable outcomes through public APIs;
+36
View File
@@ -0,0 +1,36 @@
# tests
- **Test real objects on the lightest real path.** Construct vLLM objects with
their real constructors; do not stand them in with `SimpleNamespace` or
`MagicMock`, set private attributes, or patch methods to reach a state. Mock
only external services. Pick the cheapest setup that exercises the change: a
layer or kernel test does not need `vllm_runner`, and single-rank parallel
state is enough when parallelism is irrelevant. A fix to user-visible behavior
needs at least one regression test through `LLM` (`vllm_runner`) or
`RemoteOpenAIServer`, with real weights when loading is involved. A test that
only asserts `assert_called*` or "no exception" guards nothing.
- **Show the failure before the fix.** When the bug is reproducible, run the new
test on the base commit (rebuild if the fix touches `csrc/`) and confirm it
fails for the reported reason, then passes with the fix. Paste both commands
and the relevant output in the PR; "fails before" without output is not
evidence.
- **Never buy a green CI.** Do not add `flaky`/reruns, retry loops, `skip`,
`xfail`, lowered thresholds, wider `atol`/`rtol`, or deletions to make a test
pass. Find out whether the failure is caused by your change or also happens
on main (`.buildkite/scripts/rerun-test.sh`,
`docs/contributing/ci/failures.md`); the only exception is
`xfail(strict=True)` linking an open issue for a bug on main.
- **Deterministic and exact by construction.** Use greedy or seeded sampling.
Assert equality where the result is exact (a change that must not alter
output is compared against the code it replaces, not another reference), and
keep tolerances as tight as the math allows. vLLM is not batch invariant by default: batch size, chunked
prefill, prefix caching, backend, and TP size can flip greedy tokens, so
compare against a reference with `check_logprobs_close` /
`check_embeddings_close`, or require `VLLM_BATCH_INVARIANT=1` for exact
equality. Never synchronize with a fixed `sleep`; poll an explicit condition
with a timeout, and force race orderings with events.
- **Make sure CI runs it.** A test no step in `.buildkite/test_areas/*.yaml`
selects never runs, which looks like passing. Check that the step's
`commands` and pytest markers (`core_model`, `slow_test`, `distributed`)
select the test and its `source_file_dependencies` cover the changed files;
update the step if not.