Skip to content

docs(#1802): add direct unit testing requirement for helper functions - #1803

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1802-add-helper-testing-guidance
Open

docs(#1802): add direct unit testing requirement for helper functions#1803
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1802-add-helper-testing-guidance

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

What:

Add documentation requiring direct unit tests for new public helper functions in policy/lib/.

Changes:

  • AGENTS.md: New "Testing New Helper Functions" section after the review checklist, specifying that new helpers must have direct tests covering each logical branch, edge cases, and the function contract independently of callers.
  • .claude/rules/policy-rules.md: New bullet reinforcing that indirect coverage alone is not sufficient for helper functions.

Why:

During PR #1781, human reviewer st3penta identified that the lacks_effective_anchor helper had 4 logical branches tested only indirectly through anchoring_errors. While CI's 100% coverage gate was met, the indirect tests did not guarantee all edge-case combinations were exercised. This convention exists in practice (most helpers in policy/lib/ already have direct test suites) but was not documented, so automated reviewers had no basis to flag the gap.

Tickets:

#1802


Closes #1802

Post-script verification

  • Branch is not main/master (agent/1802-add-helper-testing-guidance)
  • Secret scan passed (gitleaks — c09eff2a05a0b5ce23515a96b907b8a6f361cdfb..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Add guidance to AGENTS.md and .claude/rules/policy-rules.md requiring
new public helper functions in policy/lib/ to have direct unit tests
in their corresponding _test.rego files, not just indirect coverage
through consuming rules.

This addresses a gap identified during PR #1781 review where the
lacks_effective_anchor helper had 4 logical branches but was only
tested indirectly through anchoring_errors callers. Direct tests
should exercise each logical branch, edge cases, and the function
contract independently.

Closes #1802
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:35 AM UTC · Completed 10:46 AM UTC

Commit: 87c4a29 · View workflow run →

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unit-tests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] .claude/rules/policy-rules.md, AGENTS.md — Both modified files are under protected paths (.claude/ and AGENTS.md). The PR links to issue Add direct unit testing requirement for new helper functions to AGENTS.md #1802 and provides clear rationale for documenting the direct unit testing requirement. Human approval is always required for protected-path changes, regardless of context.

Low

  • [documentation formatting consistency] AGENTS.md:107 — The new "Testing New Helper Functions" section uses bare dash bullets (e.g., - Each logical branch of the function) whereas the checklist-style sections in AGENTS.md (Key Conventions, Common Change Patterns, Review Checklist for New Policy Rules) use the - **Bold label:** description pattern. The Rego Evaluation Model section also uses bare dashes, so this is not universally inconsistent, but the new section is closer in nature to the checklist sections.
    Remediation: Consider reformatting bullets to match the checklist pattern, e.g., - **Branch coverage:** Exercise each logical branch of the function.

Labels: PR is a documentation-only change modifying AGENTS.md and .claude/rules/policy-rules.md

@fullsend-ai-review fullsend-ai-review 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.

See the review comment for full details.

Comment thread AGENTS.md

When a PR introduces a new public helper function in `policy/lib/`, it must have
direct unit tests in the corresponding `_test.rego` file — not just indirect
coverage through consuming rules. Direct tests should exercise:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] documentation formatting consistency

The new 'Testing New Helper Functions' section uses bare dash bullets whereas the checklist-style sections in AGENTS.md (Key Conventions, Common Change Patterns, Review Checklist for New Policy Rules) use the '- Bold label: description' pattern. The Rego Evaluation Model section also uses bare dashes, so this is not universally inconsistent, but the new section is closer in nature to the checklist sections.

Suggested fix: Consider reformatting bullets to match the checklist pattern, e.g., '- Branch coverage: Exercise each logical branch of the function'.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment documentation Improvements or additions to documentation labels Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation requires-manual-review Review requires human judgment size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add direct unit testing requirement for new helper functions to AGENTS.md

0 participants