Skip to content

Docs: Add testing guidance to AGENTS.md - #18166

Merged
pvary merged 1 commit into
apache:mainfrom
pvary:docs-agents-testing
Sep 21, 2026
Merged

pvary merged 1 commit into
apache:mainfrom
pvary:docs-agents-testing

Conversation

@pvary

@pvary pvary commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Generated tests are often problematic. The fix raised in the community sync is to require a plan first: state what behavior needs verifying and why, before any test is written.

The remaining rules scope what belongs in a test, like behavior this change adds or modifies, one behavior per method, nothing for unreachable states, and nothing whose failure would not indicate a user-visible regression.


AI Disclosure

  • Model: Claude Opus 5
  • Platform/Tool: GitHub Copilot CLI
  • Human Oversight: Iterated multiple times and Fully reviewed
  • Prompt Summary: Add the test plan requirement and test scope rules raised in a community discussion on AI-generated contributions.

Generated-by: GitHub Copilot CLI (Claude Opus 5)

Generated tests are often problematic. The fix raised in the
community sync is to require a plan first: state what behavior needs
verifying and why, before any test is written.

The remaining rules scope what belongs in a test, like behavior this
change adds or modifies, one behavior per method, nothing for
unreachable states, and nothing whose failure would not indicate a
user-visible regression.

---
**AI Disclosure**
- Model: Claude Opus 5
- Platform/Tool: GitHub Copilot CLI
- Human Oversight: Iteratated multiple times and Fully reviewed
- Prompt Summary: Add the test plan requirement and test scope rules
  raised in a community discussion on AI-generated contributions.

Generated-by: GitHub Copilot CLI (Claude Opus 5)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread AGENTS.md
Comment on lines +142 to +146
- Test the behavior in the plan. Coverage is not a target.
- Test the behavior this change adds or modifies. Don't assert the correctness of components this code merely calls.
- One behavior per test method. No omnibus tests asserting several unrelated things.
- Don't add a test whose failure would not indicate a user-visible regression.
- Don't add tests for unreachable states, or tests that only restate the implementation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a good set of instructions. I would add this.

  • Do not add excessive comments in tests when the intention is clear from the test method name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this covered by #18167 in a more general way?

- Don't write a comment that restates the method name, the condition, or the next line.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Then general one works. Thanks.

@uros-b

uros-b commented Sep 21, 2026

Copy link
Copy Markdown
Member

Nice addition

@pvary
pvary merged commit cbe9bae into apache:main Sep 21, 2026
30 checks passed
@pvary

pvary commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Merged to main.
Thanks @anuragmantri, @nastra and @uros-b for the review!

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.

4 participants