test(mudblazor): allowlist the record-item model the shape guard flags (#297) - #346
Merged
Merged
Conversation
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
devis red — this unblocks it.Two PRs that never saw each other landed on
devand are individually fine but collectively red:CollectionItemShapeGuardTests— fails the build when a suitere-declares a collection-item shape
CollectionItemFixturealready provides.FieldConfigurationRefreshTestswith its ownEqualityItemModel/EqualityItem— a one-string row, which is exactlyOrderItem's shape.Each was green against the
devit branched from. Together:The fix: the allowlist, not the fixture
This is a true positive with a legitimate exception, not a guard bug — so the guard is not
weakened and the suite is not migrated.
EqualityItemis arecord, and that is the entire test.FieldConfigurationRefreshTestsreproducesthe duplicate-key render error a keyed collection loop raises for two rows comparing equal by
value — the hazard #298's README entry documents as the reason collection rows are deliberately not
keyed. Every
CollectionItemFixturemodel is a plain class with reference equality, so adoptingthe fixture would not share code, it would make the test vacuous: it would stop reproducing the
error at all. The suite's own comment already said so:
The guard compares member types, which here are identical (
String). The salient difference is thetype's equality semantics, which a shape signature deliberately does not encode — widening the
signature to capture it would make the guard miss the real copies it was built for (#258's
CredentialsModel/VaultModel). So the escape hatch #306 shipped is the right tool, used as itsown failure message directs.
Changes
EqualityItem/EqualityItemModel:private→internalso the guard test can name them, witha ⛔ comment on each saying why they must not be folded into the fixture. Visibility is not part of
the guard's ownership rule (
IsSharedShapetestsDeclaringType is null), so this changes nobehaviour — they remain nested, remain candidates, and are suppressed only by the allowlist.
DeliberateLocalCopiesallowlist onCollectionItemShapeGuardTests, carrying the reason inline.Every_Allowlisted_Type_Should_Still_Be_An_Offender_Without_The_Allowlist— new. An allowlistentry stops suppressing anything the day its suite adopts the fixture, and a stale entry is worse
than none: it would silently absolve a future re-declaration of that same type. This re-runs the
assembly-wide scan without the allowlist and requires every entry to still be a real offender,
naming any that is not. It is the same principle the guard's own header states — a check that has
quietly stopped checking still reports green.
Verification
dotnet build -c Release— clean.dotnet test -c Release— 1,616 passing, 1 skipped, 0 failing (158 Fluent / 874 core / 584 MudBlazor).MudBlazor goes 583→584: the previously-failing test passes and the new one is added.
devat65944e5, so this is verified againstthe real break rather than an assumed one.