Give the extension test projects a CI lane, and fix what it found - #4258
Merged
Merged
Conversation
#4244 fixed this for HTTP chains and stopped there. A message handler had the identical defect and kept it, and six tests in Wolverine.DataAnnotationsValidation.Tests had been failing on main to say so -- unseen, because the extension test projects run in no CI workflow. Same shape as the HTTP half: Validate<T> takes an IServiceProvider to build its ValidationContext, an unsupplied one is sourced from the service container and reported to ServiceLocationPolicy, and under the Wolverine 6 default of NotAllowed that made Wolverine's own validation middleware unusable on a message handler at all -- InvalidServiceLocationException at bootstrap. The policy supplies context.Runtime.Services itself, for the same reasons the HTTP twin supplies httpContext.RequestServices: the only thing the ValidationContext's provider ever does is answer GetService for a ValidationAttribute or an IValidatableObject, and it runs before any handler scope is relevant. Naming "context" directly is safe -- it is the generated HandleAsync parameter, and ContextVariable.OverrideName is a deliberate no-op so any other frame wanting that name is renamed instead. Wolverine.DataAnnotationsValidation.Tests: 12 passed, from 6 passed 6 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
None of them ran anywhere. TestExtensions gathers five, but only the Test and Full targets depend on it and no workflow invokes either -- dotnet.yml runs `ci` (CoreTests + CIMessageRouting) and tests.yml runs its own CI* matrix. Wolverine.Protobuf.Tests is worse: it is in wolverine.slnx so it compiles on every build, but it runs in no Nuke target at all, not even TestExtensions. VerifyCITargetCoverage did not report this and is not wrong to have missed it: it audits targets whose names begin with CI, which is the convention for "this is a lane". AiTests and its siblings never claimed to be one. Giving them a lane is the fix, and the guard covers them from here on -- it now walks 43 CI* targets and reports every one reachable. What it cost, found the moment the lane first ran: six failing tests in Wolverine.DataAnnotationsValidation.Tests, fixed in the previous commit, and Wolverine.AI.Tests red on main with the first WolverineFx.AI release in flight (#4257). Note that the AI failure surfaces here as "1 passed on retry" rather than as a failure, because the flaky-retry harness absorbs it -- the lane alone would not have made that one obvious. No docker services: none of these projects reference a persistence package or touch Servers, which is what keeps the lane cheap. CIExtensions locally: AI 40, DataAnnotations 12, FluentValidation 24, MemoryPack 3, MessagePack 5, Protobuf 1 -- all green, 27 seconds. Co-Authored-By: Claude Opus 5 <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.
Two commits: the CI lane you cannot see the value of until it runs, and the shipped bug it found in its first thirty seconds.
The gap
None of the extension test projects ran anywhere.
TestExtensionsgathers five of them, but only theTestandFulltargets depend on it, and no workflow invokes either --dotnet.ymlrunsci(CoreTests + CIMessageRouting) andtests.ymlruns its ownCI*matrix.Wolverine.Protobuf.Testsis worse still: it is inwolverine.slnxso it compiles on every build, but it runs in no Nuke target at all, not evenTestExtensions.VerifyCITargetCoveragedid not report this, and is not wrong to have missed it. It audits targets whose names begin withCI, which is the convention for "this is a lane".AiTestsand its siblings never claimed to be lanes. Giving them one is the fix; the guard covers them from here on, and now walks 43CI*targets reporting every one reachable.What it found
Six failing tests in
Wolverine.DataAnnotationsValidation.Tests, fixed in the first commit. GH-4238: DataAnnotations validation must not trip the user's ServiceLocationPolicy #4244 fixed DataAnnotations Validation not compatible with ServiceLocationPolicy.NotAllowed #4238 for HTTP chains and stopped there; a message handler had the identical defect and kept it.Validate<T>takes anIServiceProviderto build itsValidationContext, an unsupplied one is sourced from the container and reported toServiceLocationPolicy, and under the Wolverine 6 default ofNotAllowedthat made Wolverine's own validation middleware unusable on a handler at all --InvalidServiceLocationExceptionat bootstrap. The policy now suppliescontext.Runtime.Services, mirroring the HTTP twin'shttpContext.RequestServices.The CHANGELOG entry for DataAnnotations Validation not compatible with ServiceLocationPolicy.NotAllowed #4238 said DataAnnotations validation works under
NotAllowed. For handlers it did not. That entry now has a companion saying so.Wolverine.AI.Testsred on main (fixed separately in Stop Wolverine.AI.Tests racing itself through a shared static #4257), with the firstWolverineFx.AIrelease in flight.One caveat worth reading
The AI failure surfaces in this lane as "1 passed on retry", not as a failure -- the flaky-retry harness absorbs it. So the lane alone would not have made that one obvious; it took running the suite directly. Worth knowing before treating a green lane as proof that everything under it is healthy.
Verification
./build.sh CIExtensions --framework net9.0, exit 0, 27 seconds:VerifyCITargetCoveragepasses with the new lane registered. No docker services: none of these projects reference a persistence package or touchServers, which is what keeps the lane cheap enough to be uncontroversial.🤖 Generated with Claude Code