Skip to content

test(mudblazor): let every item-form builder configure its collection (#300) - #327

Merged
phmatray merged 7 commits into
devfrom
feat/300-configurecollection-exists-on-only-one-c
Aug 13, 2026
Merged

test(mudblazor): let every item-form builder configure its collection (#300)#327
phmatray merged 7 commits into
devfrom
feat/300-configurecollection-exists-on-only-one-c

Conversation

@phmatray

@phmatray phmatray commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Implements #300.

Closes #300.

Executing the implementation plan task-by-task; the checklist below — and the plan on the issue — are
ticked as each task lands. Opened as a draft — will be marked ready after the final task and a
code-review pass.

Plan

  • Task 1: Add configureCollection to the five single-field builders
  • Task 2: Add configureCollection to RootFieldAndItemForm
  • Task 3: Migrate RenderOrderForm onto TextItemForm

CI note — the one red test is pre-existing on dev, not from this PR

CollectionItemShapeGuardTests.No_Suite_Should_Re_Declare_A_Collection_Item_Shape_The_Fixture_Provides
fails on this PR and on dev itself. Measured, not inferred:

tree MudBlazor suite
origin/dev @ 65944e5 Failed: 1, Passed: 582, Total: 583
this PR merged with dev Failed: 1, Passed: 585, Total: 586

+3 tests (exactly the three self-tests added here), failures unchanged at 1. Running the guard class
against a pristine origin/dev checkout reproduces it with byte-identical offenders:

FieldConfigurationRefreshTests.EqualityItemModel holds List<EqualityItem>, whose shape (String)
  is already a CollectionItemFixture item type
FieldConfigurationRefreshTests.EqualityItemModel is a collection root whose composition (String)
  is already modelled by CollectionItemFixture.NamedOrderModel or OrderModel

Two PRs that were each green in isolation collided once both landed on dev: #306 added the guard,
#308 added FieldConfigurationRefreshTests.EqualityItemModel. Neither file is touched here.

Code review

Ten findings; eight applied, two argued down.

Applied:

  • The invoke-order contract was untested. Every self-test used AllowReorder(), an independent
    setter that reads the same whether the callback runs before or after the fixture's own
    WithLabel/WithItemForm — so all six builders could have invoked it first and stayed green. The
    spec calls that ordering the contract. Added
    Each_Item_Forms_Collection_Callback_Should_Run_After_The_Fixtures_Own_Configuration, which
    overrides the label through configureCollection across all seven builders (including
    MultiFieldItemForm, whose ordering was never pinned either).
  • Single-row models made the reorder assertion hollow. Move up renders with
    Disabled="@(index == 0)", so a one-row collection renders it permanently inert — "the button
    exists" was true of a form that cannot be reordered. Every model now carries two rows, and the
    assertion checks row 0 disabled / row 1 enabled.
  • Six copy-pasted [Fact]s → one, matching the file's own precedent
    (Each_Item_Form_Should_Apply_The_Callers_Configuration covers five builders in one test). Net −6
    copies of the same comment and assertions; per-builder coverage and negative controls unchanged.
  • RenderOrderForm still hand-rolled its render; now goes through the fixture's RenderItemForm,
    the last such wiring in a suite that uses the helper everywhere else. Verified equivalent.
  • Doc drift my own change caused: the class summary still described a one-callback contract;
    MultiFieldItemForm's remark still claimed to be the only builder reaching the collection; the six
    new parameters had no <param> docs. All corrected, with the ordering contract written down.
  • Dropped dead configuration from RootFieldAndItemForm's negative-control render.

Argued down:

  • "Extract a shared Collection<TModel,TItem> helper across the seven builders." This issue
    explicitly rejected the combinator approach (Alternatives, approach C): the fixture's value is that
    each builder is dumb and readable at the call site, and Adopt the shared collection-item fixture in the remaining test suites #258 showed unused indirection here costs
    more than the duplication. Out of scope for a parameter addition.
  • "RootFieldAndItemForm's label assertions duplicate an earlier test." They are the point of
    Task 2 Step 1 — proving a third callback did not re-route the two already there. The earlier test
    passes no configureCollection, so it cannot catch that.

Follow-ups

  • Fix the dev shape-guard breakageEqualityItemModel needs either the fixture's model or an
    entry in FindOffenders' allowlist with a reason. Deliberately not fixed here: it is outside
    this issue's test-only scope, and a fix/dev-collection-item-shape-guard branch already exists.
  • Add a conformance guard for the fixture's own surface (raised in review). Nothing stops the
    next builder added here from omitting configureCollection — which is exactly the regression
    configureCollection exists on only one CollectionItemFixture builder #300 reports, one builder later. This repo's idiom for that is a reflection test (cf.
    NativeRequiredBuilderTests): assert every IFormConfiguration<>-returning member of
    CollectionItemFixture declares a trailing optional Action<CollectionFieldBuilder<,>>. Covers
    the seven that exist and every future one; too much new surface to fold into this PR.

@phmatray
phmatray marked this pull request as ready for review August 13, 2026 13:44
@phmatray
phmatray merged commit 5569bd5 into dev Aug 13, 2026
2 checks passed
@phmatray
phmatray deleted the feat/300-configurecollection-exists-on-only-one-c branch August 13, 2026 14:20
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.

configureCollection exists on only one CollectionItemFixture builder

1 participant