[aoshen02]

Overview

Raise the bar for tests written by coding agents. Adds a 31-line tests/AGENTS.md with five rules that change what agents actually do, links it from AGENTS.md and pr-checklist §2.1, and rewrites the "unit over integration over e2e" line in AGENTS.md:

• User-visible fixes need a real-path test through LLM/vllm_runner or RemoteOpenAIServer, not only a unit test on hand-built inputs.
• Show the failure before the fix: run the new test on the base commit and paste the failing and passing output.
• Never buy a green CI: no reruns, retry loops, wider tolerances, lowered thresholds, skip, or deletion; check main first; xfail with an issue link for known bugs.
• Deterministic by construction: greedy/seeded sampling; vLLM is not batch invariant by default, so compare with check_logprobs_close/check_embeddings_close or require VLLM_BATCH_INVARIANT=1; no fixed sleep as synchronization.
• Make sure CI runs it: a test no .buildkite/test_areas/*.yaml step selects never runs and looks like a pass.

Validation

Fresh agents with no context got a repo path and one task, on main (old instructions) and on this branch.

Task	main	This PR
Sampler bug that only shows up in a batch	CPU unit test only; e2e left as a manual repro in the PR text	unit test + committed LLM regression test with an exact assertion
Fix an issue (echo=True, max_tokens=0 reports completion_tokens=1)	unit test feeding hand-built RequestOutput to the serving layer; e2e as a manual curl	regression test through the OpenAI server fixtures, streaming and non-streaming
New test fails intermittently	found the race, no retries	same, plus a negative check that the test fails without the fix
"CI is red, make it pass today"	refused to loosen the threshold	same, cites the rule; xfail + issue only if main also fails
vLLM vs HF / TP>1 bug	strong	strong; also caught that the only pooling CI step is single-GPU, so a TP=2 test would always skip

The main difference is the first rule: on main, agents fix user-visible bugs with unit tests on synthetic inputs and leave the real path untested. check_agent_files.py reports no new problems (6 existing errors in model-recipes / transformers-upgrade).

Details

The current rules come from #47784 and #57084; this replaces the "unit over integration over e2e" ordering instead of adding a conflicting rule, and keeps details in one place (tests/AGENTS.md). Prior art: sglang .claude/rules/unit-test-admission.md (regression tests fail before the fix, mock only boundaries), PyTorch review checklist (bug fixes need a reproducing test, xfail not skip), TensorRT-LLM tests/AGENTS.md (unregistered tests silently never run). Does not overlap the docs-requirement PR #62599.

cc @simon-mo @benchislett

AI assistance (Claude Code) was used; the submitter reviewed every line.

Pull Request Checklist

• I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

• AI assistance was used during the creation of this PR.

• Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

• Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

• Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

• Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[fxmarty-amd]

very nice - some personal opinions

[fxmarty-amd]

I have mixed opinion on this. vllm_runner can be quite slow in the CI (#53291) and is not always necessary.

Something like:

vllm/tests/quantization/test_online.py

Lines 1002 to 1008 in a6c898e

	# This is faster than `dist_init` arg.
	monkeypatch.setattr(
	"vllm.model_executor.parameter.get_tensor_model_parallel_rank", lambda: 0
	)
	monkeypatch.setattr(
	"vllm.model_executor.parameter.get_tensor_model_parallel_world_size", lambda: 1
	)

vllm/tests/quantization/test_online.py

Line 1804 in a6c898e

model, vllm_config = load_model_without_vllm_runner(

& generally (not on vllm-project/vllm) yet:

@pytest.fixture
def single_rank_parallel_state(monkeypatch: pytest.MonkeyPatch):
    """Single rank parallel groups without initializing a process group.

    This is faster than the `dist_init` fixture.
    """
    for name in ("_WORLD", "_TP", "_ETP", "_DCP", "_PP", "_DP", "_EP", "_PCP"):
        monkeypatch.setattr(
            f"vllm.distributed.parallel_state.{name}", _SingleRankGroup()
        )
    monkeypatch.setattr(
        "vllm.model_executor.parameter.get_tensor_model_parallel_rank", lambda: 0
    )
    monkeypatch.setattr(
        "vllm.model_executor.parameter.get_tensor_model_parallel_world_size", lambda: 1
    )

that is faster than dist_init fixture.

Instead of vllm_runner in cases tests do not read parallelism to be meaningful.

However, there are tests that need real weight loading and not only DummyModelLoader.

[fxmarty-amd]

xfail as well

[fxmarty-amd]

I would also personally prompt not to arbitrarily increase atol/rtol to make tests pass, when they are expected to have strict equality or small atol/rtol. (https://github.com/vllm-project/vllm/pull/59766)

[aoshen02]

Done

[fxmarty-amd]

LGTM

[aoshen02]

/ci run

[github-actions]

✅ Triggered Buildkite CI #96391 for commit 299761769c75.

[tomasruizt]

Nudging agents to make better tests is great!

[tomasruizt]

These should be general rules to follow and preferences, but not strict enforcements. Otherwise, agents will get stuck trying to fulfill requirements. I'm thinking concretely of:

• Objects that are really hard to instantiate atm, so MagicMock might be needed in special cases.
• Statistical tests that check a p-value, confidence intervals, code paths that check temp != 0. These seem to clash with the requirement to always run greedy/deterministic tests.

[tomasruizt]

• I like high-level tests like these, but they also have trade-offs. We could add a bullet point: "These higher-level tests are less likely to be coupled to the concrete implementation and more likely to be robust to false failure when the implementation is refactored. However, they can take many seconds or minutes to run, slowing down iteration speed. Please consider this trade-off and decide what the best level of abstraction is to introduce tests."

• And in favor of low-level unit tests: "Low-level unit tests make it much easier to quickly iterate on an implementation. You can factor out complicated logic into pure functions that are easier to unit-test. Keep in mind, that this only exercises a part of the entire system."

• Favoring test reuse over new tests: "Often the new feature can be tested by adding a new entry to a parametrized test that already exists. If the test code needs to be extended or refactored to cover the feature, that is preferable over introducing a completely new test".

[aoshen02]

follow up here: #62993