Pin every test parameter, and let a harness name the ones that drive - #61
Merged
Merged
Conversation
1.4.0 pinned only what an attribute declared, and that is the wrong default. A parameter exists to be looked at: the test was handed it so it could assert on it, or configure it and then assert on something else. Handing the test one container's instance while the invocation runs against another makes the assertion meaningless, and it is meaningless the same way for a plain application class a handler appends to as for a [Mock]. The attribute is not what makes it so. The rule is two sentences now. A test parameter is one instance for the whole test, unless it is something the harness supplies to drive the application. A registration nothing holds is per container, unless the harness pins it by name. IsolatedServices(MethodInfo) is the second half. A trigger facade, a test web application, an HttpClient and a generated client build a container per call, so pinning one pins the thing meant to be rebuilt and every call runs against one container while the test believes otherwise. Only the harness supplying them can know which they are, and it answers per test method because the answer is a property of the signature rather than of the attribute. It wins over an explicit [Shared], because that is correctness rather than preference. IServiceProvider is never pinned. A test asking for one is asking which container it is in. Found by the first test a scaffolded project runs, which takes a plain class and asserts on what the handler recorded. A rule that needs an attribute to work is a rule most tests will not get. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pinning is a statement about identity, not a reason to construct something the test never asked for. The set is drawn from a signature, so it holds types nothing registered - a value from a data row, a concrete class the resolver builds on the spot - alongside the ones that matter. The case that made this necessary is sharper than an absent registration. A harness may register a deliberately failing factory for a parameter it cannot supply, so that resolving it fails with a message naming the fix. Resolving eagerly turned that message into a failure at container build for every test that took such a parameter and never resolved it, which a data-driven test whose row supplies a string does. Twenty-four tests in the consuming framework failed that way, none of them about anything this feature touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
1.4.0 shipped the wrong default. This fixes it, and writes the rule down so it does not move a third time: What gets pinned.
What was wrong
1.4.0 pinned only what an attribute declared:
[Mock]and[InjectValues]implementISharedTestRegistration,[Shared]marks the rest. Coherent, and it survived every unit test here and 8341 in the consuming framework.It failed on the first test a scaffolded project runs. That test takes a plain application class the handler appends to and asserts on what it recorded; with no attribute on it, it was rebuilt with the container the send ran on and the test read an empty one.
A rule that needs an attribute to work is a rule most tests will not get. A bare parameter is the common case, not the exception.
The rule now
A parameter exists to be looked at, so pinning it needs no attribute.
[Shared]on a value parameter still works and is now redundant, deliberately: it earns its place on a parameter that drives the application, where it asks for one container across the calls, and a reader should not have to know which kind of parameter they are looking at before writing it.The one new member
ISharedTestRegistration.IsolatedServices(MethodInfo)names the parameters that must not be pinned, because they build containers rather than live in one: a trigger façade,ITestWebApp,HttpClient, a generated client. Only the harness supplying them can know which they are — they are ordinary types in an ordinary signature — and it answers per test method because the answer is a property of the signature rather than of the attribute.It wins over an explicit
[Shared]. Pinning one of these is not a preference that could go either way; it turns the isolation off while the test believes it is on.IServiceProvideris never pinned either. A test asking for one is asking which container it is in, which is the one question pinning cannot answer.Default implementation returning empty, so the public API diff is a single added member. Nothing removed, no signature changed.
Tests
SharedRegistrationsTestsis rewritten as one test per row of the rule, twelve of them, because the way it was wrong last time was a case nobody had written down. The container-level tests gainABareParameterIsTheSameObjectInEveryContainer, which is the shape that caught this.EachContainerGetsItsOwnApplicationSingletonnow asserts on a service the test does not take as a parameter, since taking one pins it. Without that change it would have quietly stopped testing anything.Verified
dotnet build --configuration Release -p:ContinuousIntegrationBuild=true— clean.scripts/coverage.sh 85— 90.7%.scripts/verify-packages.sh 1.5.0— real consumers on net8.0 and net10.0.1.4.0 was released hours ago and nothing consumed it, so the cost of superseding it is nil.
🤖 Generated with Claude Code