Skip to content

Split mixin loading across system and integration tiers - #1289

Merged
mkarlesky merged 2 commits into
next_versionfrom
test/mixin-tier-split
Sep 14, 2026
Merged

mkarlesky merged 2 commits into
next_versionfrom
test/mixin-tier-split

Conversation

@mkarlesky

Copy link
Copy Markdown
Member

Summary

PR 4 of 5 in the system-test redesign (Stage 2 plan, following the "System Test Duplication Assessment" report's Finding 6) -- the second-largest lever, an estimated ~5.3% of measured system-test time.

mixin_ordering_spec.rb tested precedence/dedup/ordering/sigil/:tools-append merge mechanics via a full ceedling CLI round-trip per scenario, even though that logic is pure config-hash transformation with no CLI or OS dependency -- and already has 1,970 lines of dedicated unit coverage across composinator_spec.rb, mixinator_spec.rb, mixin_resolvinator_spec.rb, recursive_merger_spec.rb, mixin_standardizer_spec.rb, and merginator_spec.rb. This is a gap left over from when system tests were the only tier available, not a design choice.

Three-way split, not a flat migration

  • spec/system/mixin_ordering_spec.rb: 36 → 7 examples. Kept only what's genuinely CLI-surface with no unit-tier analog: does --mixin/CEEDLING_MIXIN_*/a project's own :mixins config section actually get read from the real command line/environment/file, and does the pipeline exit non-zero on bad input.
  • New spec/integration/mixin_loading_spec.rb: 17 examples. A real Composinator/Projectinator/MixinResolvinator/Mixinator/MixinStandardizer/Merginator/RecursiveMerger graph, real files on a real temp directory -- no CLI subprocess, no gem deployment.
    • 10 net-new: project-directory-as-load-path (3), built-in vendor/unity/test/targets/ resolution (6), :extension yaml override (1) -- real filesystem resolution no unit spec exercises (they all stub file_wrapper/load_paths away).
    • 7 representative wiring smoke, consolidated from 19 system-tier examples whose logic is already proven exhaustively by the six unit specs above: one each for dedup, merge order, duplicate --mixin ordering, overall precedence, three-way inline-YAML positional order, the @ sigil, and the :tools arguments append exception -- proving the real object graph produces the same result end to end, not re-verifying logic those unit specs already cover scenario by scenario.

config[:history][:config] (Mixinator#mixin's own structured merge record) is used throughout the new integration spec in place of the old spec's raw log-text scanning -- a strictly more precise, directly-inspectable signal for dedup/ordering than grepping console output.

Verification

  • New integration spec: 17/17.
  • Trimmed system spec: 7/7.
  • Full rake specs:integration: 102/102 (85 existing + 17 new).
  • The six untouched mixin unit specs: 117/117 (confirms this PR touches no unit-tier files).
  • All green, host (macOS) and the madsciencelab-plugins Docker image (Linux).
  • Full-suite validation (all 16 CI matrix jobs) is CI's job per the Stage 2 plan's process -- not duplicated locally.

This is PR 4 of 5; the plan calls for a go-ahead before starting PR 5 (example-project audit + new files:/paths: spec).

🤖 Generated with Claude Code

mkarlesky and others added 2 commits September 14, 2026 15:52
PR 4 of the system-test redesign (Stage 2 plan, Finding 6) -- the
second-largest lever, ~5.3% of measured system-test time. mixin_ordering_spec.rb
tested precedence/dedup/ordering/sigil/:tools-append merge mechanics via a
full `ceedling` CLI round-trip per scenario, even though that logic is pure
config-hash transformation with no CLI or OS dependency -- and already has
1,970 lines of dedicated unit coverage across composinator_spec.rb,
mixinator_spec.rb, mixin_resolvinator_spec.rb, recursive_merger_spec.rb,
mixin_standardizer_spec.rb, and merginator_spec.rb. That's a gap left over
from when system tests were the only tier available, not a design choice.

Three-way split, not a flat migration:

- spec/system/mixin_ordering_spec.rb (36 -> 7 examples): kept only what's
  genuinely CLI-surface with no unit-tier analog -- does --mixin/
  CEEDLING_MIXIN_*/a project's own :mixins config section actually get read
  from the real command line/environment/file, and does the pipeline exit
  non-zero on bad input. Both question classes only a real `ceedling`
  subprocess can answer.

- New spec/integration/mixin_loading_spec.rb (17 examples): a real
  Composinator/Projectinator/MixinResolvinator/Mixinator/MixinStandardizer/
  Merginator/RecursiveMerger graph, real files on a real temp directory, no
  CLI subprocess, no gem deployment.
    - 10 net-new: project-directory-as-load-path (3), built-in
      vendor/unity/test/targets/ resolution (6), :extension yaml override
      (1) -- real filesystem resolution no unit spec exercises (they all
      stub file_wrapper/load_paths away).
    - 7 representative wiring smoke, consolidated from 19 system-tier
      examples whose logic is already proven exhaustively by the six unit
      specs above: one each for dedup, merge order, duplicate --mixin
      ordering, overall precedence, three-way inline-YAML positional order,
      the @ sigil, and the :tools arguments append exception -- proving the
      real object graph produces the same result end to end, not
      re-verifying logic those unit specs already cover scenario by
      scenario.

config[:history][:config] (Mixinator#mixin's own structured merge record) is
used throughout the new integration spec in place of the old spec's raw
log-text scanning -- a strictly more precise, directly-inspectable signal
for dedup/ordering than grepping console output.

Verified: new integration spec 17/17, trimmed system spec 7/7, full
`rake specs:integration` 102/102 (85 existing + 17 new), the six untouched
mixin unit specs 117/117 -- all green, host and the madsciencelab-plugins
Docker image (Linux).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's coverage-report artifact showed combined coverage dropping 5 lines
(91.209% -> 91.163%) between this PR and its predecessor. Traced via the
downloaded coverage.json artifacts from both PRs' CI runs (SimpleCov's
own per-line hit data, diffed directly) rather than guessing:

- bin/cli_handler.rb:315 (`--no-app`'s "Skipped loading Ceedling
  application" notice) was exercised *exclusively* by the 29 examples this
  PR migrated out of mixin_ordering_spec.rb -- every one of them used
  `dumpconfig --no-app` as a means to inspect merged config, and the new
  integration spec correctly bypasses the CLI entirely (calling Composinator
  directly), so --no-app itself lost its only remaining test anywhere in the
  suite. Fixed: dumpconfig_spec.rb (already the dedicated home for
  dumpconfig-as-such concerns, per its own header comment) gets one new
  example proving --no-app skips app manipulation, asserted two ways (the
  notice text, and :extension's app-load-only :source entry staying absent).

- bin/mixin_standardizer.rb:76 (re-keying a config's pre-existing String '*'
  matcher-hash key to a Symbol when promoting a mixin's plain list into it)
  was covered only incidentally, via the real temp_sensor example project's
  own already-matcher-shaped :defines section -- never a deliberately-named
  scenario in the original spec. The new integration spec's minimal
  synthetic fixture didn't reproduce that shape. Fixed: one new example in
  mixin_loading_spec.rb builds that exact shape directly and names the
  behavior explicitly, which the original spec never did.

The remaining 3 dropped lines (configurator.rb:789,
configurator_setup.rb:780-781 -- Configurator's own downstream
:plugins-validation failure path) don't trace back to this PR's diff:
:plugins isn't touched by anything mixin-related here. The original
mixin_ordering_spec.rb set ENV['CEEDLING_MIXIN_*'] directly on the real
process environment in ~10 places with no scoped cleanup (confirmed --
only 2 `after` hooks in the whole file, neither clearing those vars) --
real, pre-existing cross-example/cross-file environment leakage in this
suite's own design that predates this PR and can shift which validation
branches an unrelated, later-running spec happens to exercise from one CI
run to the next. Flagging as a suite-wide fragility worth its own future
cleanup, not something this PR's tier split caused or should chase down
further.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mkarlesky

Copy link
Copy Markdown
Member Author

Coverage investigation: why combined coverage dropped slightly

Downloaded both PRs' coverage-report CI artifacts and diffed SimpleCov's per-line hit data directly (PR #1288's run vs. this PR's run) rather than guessing:

Combined: 91.209% → 91.163% (-0.046pt), 5 lines out of 10,795 total instrumented lines.

File:line Explanation Status
bin/cli_handler.rb:315 --no-app's "Skipped loading Ceedling application" notice — exercised exclusively by the 29 examples migrated out of mixin_ordering_spec.rb; the new integration spec correctly bypasses the CLI entirely Fixed — new example in dumpconfig_spec.rb
bin/mixin_standardizer.rb:76 Re-keying a config's String '*' matcher-hash key to a Symbol — covered only incidentally via the real temp_sensor project's pre-existing matcher-shaped :defines, never a deliberately-named scenario Fixed — new example in mixin_loading_spec.rb
configurator.rb:789, configurator_setup.rb:780-781 Configurator's own :plugins-validation failure path — doesn't trace back to this PR's diff (:plugins isn't touched by anything mixin-related here) Not fixed here — see below

The last 3 lines are more likely a symptom of a pre-existing suite fragility this PR surfaced rather than caused: the original mixin_ordering_spec.rb set ENV['CEEDLING_MIXIN_*'] directly on the real process environment in ~10 places with no scoped cleanup (confirmed — only 2 after hooks in the whole file, neither clearing those vars). That's real cross-example/cross-file environment leakage that can shift which validation branch an unrelated, later-running spec happens to exercise from one CI run to the next, independent of anything this PR changed. Flagging it as a separate, suite-wide cleanup candidate rather than chasing it down further in this PR.

Latest commit (9186564) adds the two fixable cases. New totals: integration spec 17→18 examples, dumpconfig_spec.rb +1 example.

@mkarlesky
mkarlesky merged commit d8f05cb into next_version Sep 14, 2026
20 checks passed
@mkarlesky
mkarlesky deleted the test/mixin-tier-split branch September 14, 2026 21:34
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