Skip to content

Invalidate the message-name cache when naming strategies change (GH-3703) - #3717

Merged
jeremydmiller merged 1 commit into
mainfrom
fix/3703-naming-cache-invalidation
Jul 29, 2026
Merged

Invalidate the message-name cache when naming strategies change (GH-3703)#3717
jeremydmiller merged 1 commit into
mainfrom
fix/3703-naming-cache-invalidation

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #3703.

What was wrong

WolverineMessageNaming.ToMessageTypeName memoizes into a static ImHashMap<Type, string> (src/Wolverine/Util/WolverineMessageNaming.cs:140), and WolverineRuntime.HostService.StartAsync pre-populates it from Handlers.AllMessageTypes() on every host start. Registering an interop assembly afterwards mutated the InteropAssemblyInterfaces strategy but left every already-resolved name in the map, so a type named before the registration kept its old name forever and silently ignored the new strategy.

CoreTests boots hundreds of hosts, so by the time WolverineMessageNamingTests.use_interface_from_interop_message_naming ran, CoreTests.Util.ConcreteMessage had already been cached under its plain full name — and the test's AddMessageInterfaceAssembly could not displace it. It failed deterministically in the full suite while passing alone.

This is a real (if narrow) product bug, not just a test artifact: IPolicies.RegisterInteropMessageAssembly is documented as the way to make Wolverine forward message names to NServiceBus/MassTransit interfaces, and it silently did nothing for any type already named in that process.

The fix

AddMessageInterfaceAssembly and InsertFirst<T> clear the memo. Both are configuration-time calls — the only product caller is IPolicies.RegisterInteropMessageAssembly — and both now no-op when the registration would not change anything, so the hundreds of host starts in a test run do not thrash the cache. The map is a pure memo, so discarding it only costs a recompute.

Verification

  • CoreTests: 2104 total / 2102 passed / 0 failed / 2 skipped, up from the documented 2101 passed / 1 failed baseline on main — exactly the expected one-test delta, no other movement.
  • WolverineMessageNamingTests alone: 8/8.
  • dotnet build wolverine.slnx -c Release: succeeded.

No new test was added. A second test exercising the same global static would itself have been order-dependent on whichever test registered the assembly first; the existing test is the regression guard and is now deterministic in both directions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JMNKwGVHnyaBjiheC5k8KX

… change (GH-3703)

`WolverineMessageNaming.ToMessageTypeName` memoizes into a static `ImHashMap`, and
`WolverineRuntime.HostService.StartAsync` pre-populates it from
`Handlers.AllMessageTypes()`. Registering an interop assembly afterwards mutated the
`InteropAssemblyInterfaces` strategy but left every already-resolved name in place, so
any type named before the registration kept its old name forever and silently ignored
the new strategy.

`CoreTests` boots hundreds of hosts, so by the time
`WolverineMessageNamingTests.use_interface_from_interop_message_naming` ran,
`CoreTests.Util.ConcreteMessage` had already been cached under its full name and the
test failed deterministically in the full suite while passing alone.

`AddMessageInterfaceAssembly` and `InsertFirst<T>` now clear the cache. Both are
configuration-time calls -- the only product caller is
`IPolicies.RegisterInteropMessageAssembly` -- and both no-op when the registration
would not change anything, so the hundreds of host starts in a test run do not thrash
the cache. The map is a pure memo, so discarding it only costs a recompute.

Verified: CoreTests goes from 2101 passed / 1 failed to 2102 passed / 0 failed
(2104 total, 2 skipped), matching the documented baseline delta exactly.
`dotnet build wolverine.slnx -c Release` succeeded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JMNKwGVHnyaBjiheC5k8KX
@jeremydmiller
jeremydmiller merged commit 39f900c into main Jul 29, 2026
31 checks passed
jeremydmiller added a commit that referenced this pull request Jul 29, 2026
* test: reinstate the xUnit1051 analyzer across 71 of 73 xUnit projects (GH-3702)

xUnit1051 ("use TestContext.Current.CancellationToken") was suppressed repo-wide in
Directory.Build.props during the v3 migration, because TreatWarningsAsErrors turned it
into a hard build break across 2,264 call sites (8,140 raw warnings once multi-targeting
is counted).

The fix is applied by xUnit's OWN code fix, not by hand and not by a regex. `dotnet format
analyzers` cannot drive it -- Xunit.Analyzers.Fixes.UseCancellationTokenFixer returns null
from GetFixAllProvider(), and dotnet format only applies fixers that support FixAll -- so a
small Roslyn driver loads the analyzer and invokes the CodeFixProvider directly, one
compilation per pass, merging the disjoint TextChanges per file.

Three things that driver had to account for, each of which a blind sweep gets wrong:

- The tool's own diagnostic count cannot be trusted. An MSBuildWorkspace load on a cold
  project can return an incomplete compilation and report ZERO diagnostics -- CoreTests
  first reported 0, then 472 on retry. The build, with xUnit1051 back at error severity,
  is the gate; the driver re-runs the fixer while anything is still reported.

- The fixer sometimes binds to the wrong named parameter. On
  `InvokeAsync<T>(object, CancellationToken, TimeSpan?)` it emitted
  `timeout: TestContext.Current.CancellationToken`. CS1503 caught both occurrences.

- It declines some shapes outright: `Task.Run(() => ...)` and
  `Task.WhenAny(tcs.Task, Task.Delay(...))`. Twelve sites threaded by hand.

Separately, the analyzer fires inside NSubstitute verifications, where taking its advice is
actively wrong: `channel.Received().QueueDeclareAsync(..., cancellationToken: TestContext
.Current.CancellationToken)` narrows the verification to that exact token, which production
code does not pass. Those 25 sites use `Arg.Any<CancellationToken>()` instead, which the
analyzer accepts.

Rather than a global flag flip, Directory.Build.targets carries a conditional NoWarn so each
project opts in with

    <XUnitCancellationTokenEnforced>true</XUnitCancellationTokenEnforced>

It has to live in .targets because the property is set by the project file, which is
evaluated after Directory.Build.props. Delete the block once the last two projects are in.

71 of 73 xUnit projects are converted. The two left out are SampleTests and TracingTests,
which do not compile at all on main -- that is GH-3704.

Verified: `dotnet build wolverine.slnx -c Release` succeeds. CoreTests runs 2104 total /
2101 passed / 1 failed, the same single pre-existing failure main has (GH-3703, fixed
separately in #3717) -- i.e. no movement from this change. Remaining suites are
compile-verified only and rely on CI; anything that misbehaves will do so at runtime, not
at build time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JMNKwGVHnyaBjiheC5k8KX

* fix(tests): match any CancellationToken in NSubstitute arranges, not the test's (GH-3702)

The xUnit1051 sweep threaded `TestContext.Current.CancellationToken` into NSubstitute
mock interactions. On a verification that is merely narrowing, and on an arrange it is
worse than narrowing: it makes the stub match only that exact token. Production code
passes its own token -- `default` in every one of these paths -- so the arranged call
never matches, NSubstitute hands back `null`, and the test dies dereferencing it.

That is the 11 consistently-failing tests on this PR's last CI run, all of them
`NullReferenceException` under a transport's endpoint-initialization unit tests:

  Wolverine.AmazonSns.Tests      when_initializing_the_endpoint            (1)
  Wolverine.AmazonSqs.Tests      when_initializing_the_endpoint            (4)
  Wolverine.AzureServiceBus.Tests AzureServiceSubscriptionTests            (1)
  Wolverine.RabbitMQ.Tests       Internals.RabbitMqQueueTests              (5)

The original commit already knew verifications had to use `Arg.Any<CancellationToken>()`
and converted 25 of them; it just did not carry the same rule to the arrange side. The
rule is simply: NSubstitute interactions match on arguments, so a mock call -- arrange
or assert -- takes `Arg.Any<CancellationToken>()`. Never the ambient test token.

13 sites across 5 files. Found by tokenizing every file into statements and flagging any
`TestContext.Current.CancellationToken` inside a statement carrying an NSubstitute marker
(`.Returns`/`.Received`/`Arg.*`/...). A second, independent pass -- flag the token whenever
it is an argument to a call on a variable the file builds with `Substitute.For<>` -- now
reports zero, so this clears the class and not just the failures CI happened to surface.

Verified: red baseline reproduced locally before the fix (Wolverine.AmazonSqs.Tests
`when_initializing_the_endpoint` = 4 failed / 4 passed, same 4 as CI), then 0 failed /
8 passed after. All four projects build with xUnit1051 at error severity, 0 warnings.
SNS 2/2, SQS 8/8, ASB 5/5, RabbitMQ 25/25 (includes native_dead_letter_queue_mechanics
against a live broker, whose two sites were latent -- they had not yet failed on CI).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

CoreTests: use_interface_from_interop_message_naming is order-dependent and fails on main

1 participant