Skip to content

[Doc] Make agent test rules defaults with stated exceptions - #62993

Open
aoshen02 wants to merge 1 commit into
vllm-project:mainfrom
aoshen02:docs/agents-test-defaults
Open

aoshen02 wants to merge 1 commit into
vllm-project:mainfrom
aoshen02:docs/agents-test-defaults

Conversation

@aoshen02

Copy link
Copy Markdown
Contributor

Purpose

Follow-up to #62601, addressing @tomasruizt's review (#62601 (review)).

  • State that the rules in tests/AGENTS.md are defaults: an agent may deviate when one does not fit, and must say why in the PR. Weakening a test to make it pass stays forbidden.
  • Add the trade-off between end-to-end tests (robust to refactors, slow) and unit tests on pure functions (fast, partial coverage), and encourage factoring logic into pure functions.
  • Allow mocks for objects that are impractical to construct, with the reason stated in the test.
  • Allow seeded statistical assertions (p-value, confidence interval) when sampling itself (temperature > 0) is under test.
  • In AGENTS.md, prefer adding a case to an existing parametrized test, then extending existing tests, before writing a new one.

Test Plan

Docs-only change. Ran markdownlint-cli2 with the repo config on both files. Gave fresh agents with no prior context (a Claude subagent and Kimi) four tasks against this checkout: a top-p edge-case fix, a min-p distribution change at temperature 0.8, a helper inside the KV cache manager, and a 1-in-5 CI flake with an atol mismatch.

Test Result

markdownlint-cli2: 0 issues. Both agents added a case to the existing sampler test file, used a seeded chi-square test for the sampling change, tested the KV cache helper as a pure function or through the existing real-manager fixtures instead of mocks, and refused to widen atol, add reruns, or xfail for the flake.

AI assistance (Claude Code) was used to draft this change; the submitter reviewed every changed line.

🤖 Generated with Claude Code

Follow-up to vllm-project#62601 review: treat the rules as defaults that can be
deviated from with a stated reason, spell out the trade-off between
end-to-end and unit tests, allow mocks for objects that are impractical
to construct, allow seeded statistical assertions for sampling tests,
and prefer adding a case to an existing parametrized test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: aoshen02 <281812371+aoshen02@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mergify mergify Bot added the agents Related to information for agents label Oct 21, 2026

@tomasruizt tomasruizt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for including these!

@fxmarty-amd fxmarty-amd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Related to information for agents

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants