Skip to content

rules: document tests outside rules/is, prefer plain rules - #911

Open
samlown wants to merge 1 commit into
mainfrom
rules-docs-helper-tests
Open

rules: document tests outside rules/is, prefer plain rules#911
samlown wants to merge 1 commit into
mainfrom
rules-docs-helper-tests

Conversation

@samlown

@samlown samlown commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Docs-only change to rules/README.md, prompted by review of gobl.dk.oioubl#8, where a large number of is.Func helpers were written for checks the framework and domain packages already cover.

Tests exist outside rules/is

The "Available tests" section only documented rules/is, so there was nothing pointing addon authors at the tests each domain package already exports. Added a preference order (generic → domain → custom function) and a new Domain tests outside rules/is section inventorying every exported rules.Test in the repo:

  • numPositive, ZeroOrPositive, NotZero, Min/Max/Equals, .Exclusive()
  • calDateNotZero/After/Before plus the DateTime and Timestamp equivalents
  • cbcInCodes, InCodeDefs, InKeyDefs, HasValidKeyIn, CodeMapHas
  • tax — the six Extensions* rules, ExtensionHasValidCode, SetHasCategory/SetHasOneOf, RegimeIn, IdentityIn, HasAddon/AddonIn, TagsIn
  • org, bill, currency, head, uuid — identity and attribute tests, the *TypeIn When guards, RequireLineTaxCategory, CanConvertTo, StampsHas, UUID version tests

Closes with a note asking authors to push generally-useful tests down into the type's own package rather than keeping them private to an addon.

Prefer plain rules over helper functions

New section leading "Common patterns", with three before/after pairs drawn from the patterns in that PR:

  • is.Func that type-asserts to reach a nested value → nested rules.Field + AssertIfPresent(is.In(...))
  • hand-rolled amount and tax-set predicates → num.NotZero, tax.SetHasCategory
  • one helper spanning two fields → one assertion per field, so faults land on totals.payable and totals.due separately instead of collapsing onto totals

The point worth the most: a helper's val.(*bill.Tax) guard returns true on a type mismatch, so a field rename silently disables the rule with no test failure, whereas Field validates its name against the struct at initialisation and panics. The section ends by saying custom functions remain the right answer for genuine cross-field logic and domain lookups.

All three "Prefer" examples were compile-run against real bill.Invoice data before being documented — the five rules fire with exactly the codes and paths shown, and an empty rounding is correctly skipped.

Fault format correction

Found while verifying the above: the Testing section documented faults.HasPath("field") and an error format of [CODE] field: description. HasPath matches JSON Paths rooted at $ exactly, and Fault.Error renders [CODE] ($.field) description. Corrected both, and documented that faults sharing a code and message merge into one entry listing every path.

No CHANGELOG entry — docs only.

🤖 Generated with Claude Code

Add an inventory of the rules.Test implementations exported by num, cal,
cbc, tax, org, bill, currency, head and uuid, and state the preference
order: generic tests, then domain tests, then a custom function.

Add a section showing nested Field, num and tax tests, and per-field
assertions in place of custom is.Func helpers.

Correct the documented fault path format to the JSON Paths rooted at $
that HasPath matches, and note that faults sharing a code and message
merge into one entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 10:37

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@samlown
samlown requested review from alvarolivie and cavalle July 28, 2026 10:40
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.87%. Comparing base (43ca367) to head (d922f50).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #911   +/-   ##
=======================================
  Coverage   94.87%   94.87%           
=======================================
  Files         320      320           
  Lines       17431    17431           
=======================================
  Hits        16537    16537           
  Misses        553      553           
  Partials      341      341           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants