Guard the DI container's orderings and shape - #2119
Draft
NickJosevski wants to merge 1 commit into
Draft
Conversation
Script wrappers form an execution chain and file-format replacers are tried in sequence, so a reordering changes deployment behaviour. Nothing currently detects that: every wrapper still constructs and every command still resolves, so the existing suite stays green. Two guards, both PlatformAgnostic: ContainerOrderingFixture asserts the orderings directly. The load-bearing one is ScriptWrapperPrioritiesAreUnique — ScriptEngine sorts with OrderByDescending, which is stable, so a shared priority silently hands the tie-break to the container's collection ordering. Priorities are distinct today; the test keeps them that way. The rest pin the wrapper chain for a Kubernetes/AWS/PowerShell step, the file-format replacer order, and discoverer key uniqueness. ContainerSnapshotFixture compares a rendering of the whole container against an approved file: registrations, lifetimes, collection order, and the concrete types injected into every command. Targeted assertions only catch what someone thought of; this catches anything observable that moves, which is what makes it useful across a dependency upgrade. Re-approve with CALAMARI_APPROVE_CONTAINER_SNAPSHOT=1. The snapshot is normalised so one approved file serves every platform: the OS-chosen filesystem and certificate-store implementations collapse to placeholders, Autofac's __RegistrationOrder tick counter is dropped, and collection fields render distinct element types rather than counts. Verified against the Autofac 4.8.0 -> 9.3.1 upgrade in #2095: the snapshot fails with exactly the version line plus five registrations gaining AutoActivate (the RegisterInstance ones), and all five ordering tests still pass.
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
1 participant
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.
Adds two DI guards to
Calamari.Tests. No production code changes.Why
Script wrappers form an execution chain; file-format replacers are tried in sequence. A reordering changes deployment behaviour, but every wrapper still constructs and every command still resolves — so the existing suite stays green. Nothing currently detects it.
What
ContainerOrderingFixture— asserts the orderings directly. The load-bearing test isScriptWrapperPrioritiesAreUnique:ScriptEnginesorts withOrderByDescending, which is stable, so two wrappers sharing a priority silently hand the tie-break to the container's collection ordering. Priorities are distinct today; this keeps them that way. The rest pin the wrapper chain for a Kubernetes/AWS/PowerShell step, the replacer order, and discoverer key uniqueness.ContainerSnapshotFixture— compares a rendering of the whole container againstContainerSnapshot.expected.txt: registrations, lifetimes, collection order, and the concrete types injected into every command. Targeted assertions only catch what someone thought of; this catches anything observable that moves, which is what makes it useful across a dependency upgrade. Re-approve withCALAMARI_APPROVE_CONTAINER_SNAPSHOT=1.Both are
PlatformAgnostic. The snapshot is normalised so one file serves every platform — OS-chosen filesystem and certificate-store implementations collapse to placeholders, Autofac's__RegistrationOrdertick counter is dropped, collection fields render distinct element types rather than counts.The expected file is deliberately not named
*.approved.*—.gitattributes:51marks that pattern binary, which would hide the diff this test exists to surface.Verified
Simulated the Autofac 4.8.0 → 9.3.1 bump from #2095 on this branch. The snapshot fails with exactly the version line plus five registrations gaining
AutoActivate— theRegisterInstanceones — and all five ordering tests still pass. Reverted before committing.Baselined on
mainat Autofac 4.8.0, so when #2095's stack rebases the upgrade produces that diff for a reviewer to accept.