tests: generic conformance checks over every material (Tier 1 of #35) - #37
tests: generic conformance checks over every material (Tier 1 of #35)#37petlenz wants to merge 4 commits into
Conversation
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
left a comment
There was a problem hiding this comment.
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_stepperexemption is genuine — registered as_rank1/_rank2— and was found by the check itself. - CI green.
Part of #35 — Tier 1 only. Stacked on the #33 fix, which it would otherwise fail.
Five checks, none needing a per-material fixture:
material_baseis registered, or on an explicit opt-out list with its reasonnameparameters()that forgot to chainbase::parameters())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. Thetensor_component_stepperexemption 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 fixturetable and are not in this PR.