Skip to content

tests: generic conformance checks over every material (Tier 1 of #35) - #37

Open
petlenz wants to merge 4 commits into
fix/register-materialsfrom
test/generic-material-suite
Open

tests: generic conformance checks over every material (Tier 1 of #35)#37
petlenz wants to merge 4 commits into
fix/register-materialsfrom
test/generic-material-suite

Conversation

@petlenz

@petlenz petlenz commented Aug 22, 2026

Copy link
Copy Markdown
Member

Part of #35 — Tier 1 only. Stacked on the #33 fix, which it would otherwise fail.

Five checks, none needing a per-material fixture:

  • every class deriving material_base is registered, or on an explicit opt-out list with its reason
  • the opt-out list has no stale entries, so an exemption cannot outlive the material it exempts
  • every schema declares name
  • no schema is empty (the signature of a parameters() that forgot to chain base::parameters())
  • a missing required parameter throws

The opt-out list is the point

Three materials are deliberately absent because they are templates registered under concrete aliases. Each is now a decision someone wrote down, instead of being indistinguishable from an oversight — which is exactly how #33 happened.

Verified load-bearing

Deleting one register_type() line fails the check, naming the material and the fix. The tensor_component_stepper exemption was itself found by the check on its first run — it is registered under _rank1/_rank2, which the check could not know.

Tiers 2 and 3 need a type -> minimal JSON fixture table and are not in this PR.

Registration and schema declaration are both hand-maintained lists, and a
missing entry is silent: the material still works from C++ and is simply absent
from the document layer. Eight materials were unreachable that way (#33), found
by auditing rather than by any test.

Five checks, none needing a per-material fixture:

  - every class deriving material_base is registered, or is on an explicit
    opt-out list WITH its reason
  - the opt-out list has no stale entries, so an exemption cannot outlive the
    material it exempts
  - every schema declares "name"
  - no schema is empty (the signature of a parameters() that forgot to chain
    base::parameters())
  - a missing required parameter throws

The opt-out list is the point. Three materials are deliberately absent because
they are templates registered under concrete aliases -- small_strain_plasticity
and rk_plasticity under j2_plasticity/drucker_prager_plasticity/j2_rk_plasticity,
tensor_component_stepper under _rank1/_rank2. Each is now a decision written
down, instead of being indistinguishable from an oversight.

Verified load-bearing: deleting one register_type() line fails the check by
name with the fix in the message. The tensor_component_stepper exemption was
itself found by the check on its first run.

Part of #35. Tiers 2 and 3 need a type -> minimal JSON fixture table and are
not in this change.
The first version asserted nothing. It caught the exception, ignored both
outcomes, and checked only that the loop had run -- so it passed whether or not
any material enforced its required parameters. In a PR whose whole purpose is
catching that class of test, written by me.

The cause was real and is now stated: is_required is a check attached with
.add<is_required>() and the controller exposes no way to ask whether a
parameter is required, so a loop over every registered type cannot distinguish
'threw because something was required' from 'succeeded because nothing was'.
Rather than solve that, the first version kept the loop and dropped the
assertion.

Now it names five types with known-required parameters and asserts each throws
when given a document with only 'name'. Not generic, and the comment says why,
with the condition under which it becomes generic again.

Verified load-bearing: swapping linear_stress's is_required for a default makes
it fail, naming the material.

@petlenz petlenz left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Critical review of my own PR. One finding: a test in it asserted nothing. Fixed in the branch.

MissingRequiredParametersThrow could not fail

try {
  nm::create_from_json<policy>(ctx, j);
  // Reaching here is fine only if nothing was actually required.
} catch (const std::exception&) {
  // Threw, which is the contract.
}

Both branches empty. The only assertion was checked > 0 — that the loop had run. It passed whether or not any material enforced a required parameter, in the PR whose entire purpose is catching that class of test.

The cause was real: is_required is a check attached with .add<is_required>(), and the controller exposes no way to ask whether a parameter is required. A loop over every registered type genuinely cannot distinguish "threw because something was required" from "succeeded because nothing was". Faced with that, the first version kept the loop and quietly dropped the assertion instead of narrowing the claim.

It now names five types with known-required parameters and asserts each throws on a document containing only name. Not generic, and the comment says so, along with the condition that would make it generic again — if the controller ever exposes required-ness, the list goes and the loop returns.

Verified load-bearing: swapping linear_stress's is_required for a default fails it, naming the material.

What I checked and could not break

  • The registration check fails, by name, when any single register_type() line is deleted.
  • The stale-exemption check works: an opt-out entry for a material that no longer exists fails.
  • The tensor_component_stepper exemption is genuine — registered as _rank1/_rank2 — and was found by the check itself.
  • CI green.

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.

1 participant