Skip to content

GH-4559: make the compliance tests ask the broker, not Wolverine's configuration objects - #4579

Merged
jeremydmiller merged 1 commit into
mainfrom
gh-4559-compliance-asserts-broker-state
Sep 23, 2026
Merged

jeremydmiller merged 1 commit into
mainfrom
gh-4559-compliance-asserts-broker-state

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #4559.

The shape of the bug

all_queues_are_declared_as_quorum passed for as long as it existed, over a queue that was classic:

queues.Any().ShouldBeTrue();
foreach (var mqQueue in queues)
{
    mqQueue.QueueType.ShouldBe(QueueType.quorum);   // Wolverine's own config object
}

QueueType is the value Wolverine set on its own endpoint object during configuration. The assertion is true by construction — it would pass against a broker that was switched off. ProcessInlineFixture had copy-pasted .DeclareQueue("quorum1") from QuorumQueueFixture and created that queue as classic; whichever class ran second got a 406 inequivalent arg 'x-queue-type' that the old tolerate-and-continue path swallowed, and the quorum suite then ran its whole 23 tests against a classic queue with this assertion still green. #4541 fixed the copy-paste. This fixes why nothing noticed.

Asking the broker

There is no AMQP way to ask any of this: QueueDeclarePassiveAsync returns only the queue name, message count and consumer count, so the arguments a queue was actually created with are never on the wire. RabbitManagementProbe — already in the suite for GH-4095's broker-side kills — grows four readers over the management API, which ships in the rabbitmq:4-management image the compose file already pins:

Method Answers
GetQueueTypeAsync classic / quorum / stream, off the effective top-level type
GetQueueArgumentsAsync the arguments the broker holds, x-dead-letter-exchange included
GetBoundExchangesAsync the exchanges bound to a queue
ExchangeExistsAsync whether an exchange is really there

plus RequireAsync, which throws rather than skipping when port 15672 is unreachable — a test that skips here quietly goes back to proving nothing.

What was rewritten

The audit in item 2 turned up six more instances of the same shape. All of them are in:

RabbitMQ — quorum_queue_compliance, stream_queue_compliance (the exact unmodified twin), Bug_3871 ..._declare_consistently, sets_the_dead_letter_queue_exchange_on_created_queues, both publish-side ..._during_auto_provision tests, interop_friendly + wolverine_storage ..._dead_letter_queue_exchange_on_created_queues, and Bug_2681.both_message_exchanges_were_created.

default_and_override_queues_keep_their_own_dlx_exchange_on_declare drives DeclareAsync against an NSubstitute channel and then asserted the endpoint's own Arguments — so the declare was incidental. It now asserts on what went out over the channel.

Redis — disabled_dead_letter_queue_should_not_create_dead_letter_stream asserted NativeDeadLetterQueueEnabled, the flag DisableNativeDeadLetterQueue() had set two lines of configuration earlier, and never failed a message, so nothing could have created the stream either way. It now fails one and asks Redis.

SQS — the three default_dead_letter_queue_name tests whose names promise provisioning read transport.Queues, Wolverine's endpoint cache. They now call ListQueues. global_disable_trumps_transport_default_during_provisioning also ran without AutoProvision(), so there was never anything for the kill switch to suppress; it does now.

Three negative claims stay at the configuration level on purpose, each with a comment saying why: "wolverine-dead-letter-queue" is a fixed name other tests legitimately create on the same LocalStack, so its presence there says nothing about the host under test.

Shared fixed names (item 3, the one with a deadline)

quorum1/quorum2, inline1/inline2 and stream1/stream2 now come from RabbitTesting.NextQueueName() — unique per process and per call, so no fixture can declare another's queue with a different shape and no run can inherit one. Static per fixture, because TransportCompliance<T> does new T() and xUnit builds a new test class instance per test method; per-instance names would leave 46 durable queues on the broker per run.

sending_raw_messages already used generated names, so its "skipping in CI via the Flaky filter" marker and its #2618 deadline were stale — no [Trait("Category", "Flaky")] on the class either. Replaced with a note not to reintroduce a fixed name.

Dead coverage

when_discovering_a_sender_with_all_defaults carried a commented-out has_bound_the_exchange_to_a_queue_of_the_same_name — the one test in that file that would have covered bindings, dead long enough to still refer to theRuntime/theTransport as properties. It cannot hold in that fixture anyway: DisableListenerDiscovery is set, so the convention only creates the exchange and there is no queue to bind. The coverage moved to when_discovering_a_listening_endpoint_with_all_defaults, where the broker's own binding list is asked.

Verification

Every rewritten assertion was run against a deliberately broken control, because a test that cannot fail is exactly what this issue is about:

  • UseQuorumQueues() removed from both hosts → the quorum test fails with the broker's "classic" against the expected "quorum". The old assertion still passed.
  • Wolverine's queue.Arguments[x-dead-letter-exchange] overwritten with nonsense after startup → the DLX tests still pass, which is the proof they are no longer reading it. The old assertion would have failed.
  • Both DeclareAsync calls deleted → the on_declare test fails. It passed before.
  • The SQS existence helper aimed at a queue that does exist → ShouldBeFalse fails.
  • Redis DLQ re-enabled → KeyExistsAsync is true and the test fails.

Test runs: full Wolverine.RabbitMQ.Tests (538), Redis DeadLetterQueueTests (6), SQS default_dead_letter_queue_name (10). Pinned full-solution build clean: dotnet build wolverine.slnx -c Release -f net9.0.

One pre-existing flake showed up under full-suite load — connection_recovery_after_a_broker_side_kill.listeners_resume_after_the_connection_is_killed_underneath_them timed out, and passes in isolation. It touches nothing in this change; it depends on the management plugin's stats collector keeping up, which is the lag its own comments describe.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

…nfig objects

A test named "all queues are declared as quorum" that reads RabbitMqQueue.QueueType
back off Wolverine's own endpoint object cannot fail: QueueType is the value
Wolverine set during configuration, so the assertion is true by construction and
would pass against a broker that was switched off. It did pass, for as long as
ProcessInlineFixture was declaring that same queue as classic and the old
tolerate-and-continue path was swallowing the broker's 406.

Every assertion here now comes from the broker, and every test whose name promises
that something was created, declared or provisioned asks the thing that would know.

RabbitManagementProbe grows GetQueueTypeAsync, GetQueueArgumentsAsync,
GetBoundExchangesAsync and ExchangeExistsAsync, plus a RequireAsync factory that
fails loudly rather than skipping. AMQP genuinely cannot answer any of this --
QueueDeclarePassiveAsync returns only the name, message count and consumer count --
so the management plugin, which ships in the rabbitmq:4-management image the compose
file already pins, is the only source.

Reworked to read real state:

  - quorum_queue_compliance.all_queues_are_declared_as_quorum
  - stream_queue_compliance.all_queues_are_declared_as_stream (the exact twin)
  - Bug_3871 ..._declare_consistently
  - native_dead_letter_queue_mechanics: sets_the_dead_letter_queue_exchange_on_created_queues,
    both publish-side ..._during_auto_provision tests, and
    default_and_override_queues_keep_their_own_dlx_exchange_on_declare, which now
    asserts on what went out over the channel (it would have passed with both
    DeclareAsync calls deleted)
  - interop_friendly / wolverine_storage ..._dead_letter_queue_exchange_on_created_queues
  - Bug_2681.both_message_exchanges_were_created
  - Redis DeadLetterQueueTests.disabled_dead_letter_queue_should_not_create_dead_letter_stream,
    which asserted the flag DisableNativeDeadLetterQueue() had just set and never
    failed a message at all
  - SQS default_dead_letter_queue_name: the three tests whose names say "provisioned"

Shared fixed queue names are gone: quorum1/quorum2, inline1/inline2 and
stream1/stream2 now come from RabbitTesting.NextQueueName(), which is unique per
process and per call, so no fixture can declare another's queue with a different
shape and no run can inherit one. The names are static per fixture because xUnit
builds a new fixture per test method. sending_raw_messages already used generated
names, so its "skipping in CI via the Flaky filter" marker was stale -- replaced
with a note not to reintroduce a fixed name.

The conventional-routing sender fixture had a commented-out
has_bound_the_exchange_to_a_queue_of_the_same_name -- the one test in that file that
would have covered bindings, and dead long enough to still refer to
theRuntime/theTransport as properties. It cannot hold there anyway
(DisableListenerDiscovery means no queue exists to bind), so the coverage moved to
when_discovering_a_listening_endpoint_with_all_defaults, where the broker's own
binding list is asked.

Each rewritten assertion was checked against a deliberately broken control: with
UseQuorumQueues() removed the quorum test reports the broker's "classic"; with
Wolverine's Arguments dictionary overwritten with nonsense the DLX tests still pass,
which is the proof they are no longer reading it.

Three negative claims stay at the configuration level on purpose, each with a
comment: "wolverine-dead-letter-queue" is a fixed name other tests legitimately
create on the same LocalStack, so its presence there says nothing about the host
under test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
@jeremydmiller
jeremydmiller merged commit 29f5fcd into main Sep 23, 2026
43 checks passed
jeremydmiller added a commit that referenced this pull request Sep 24, 2026
… test suites (#4597)

Extracted from @tmorejon's #4506, which found these while building a validator for GH-4059 and is the
original work here. The core fix landed separately as #4582 with a WARNING rather than a rejection --
"send inline, receive durably" is a coherent thing to want that a single Endpoint.Mode cannot express --
but these 27 files are real misconfigurations either way, and #4582's warning now reports every one of
them.

Most are a `.SendInline()` on a publish rule for an endpoint whose listener separately asked for
UseDurableInbox() or ProcessInParallelWithNativeAcks(); whichever configuration block Wolverine applied
last silently decided the mode for both directions.

The Redis dead letter tests are the interesting ones, and the reason this is worth landing rather than
leaving as noise: they were RELYING on the bug. Their listener needs Inline, because only an Inline
listener takes RedisStreamListener.MoveToErrorsAsync -- a buffered one dead-letters through
TryBuildDeadLetterSender instead -- and it was silently getting that from the SendInline() on the
publishing side sharing Endpoint.Mode. They now say ProcessInline() outright.

Verified: Wolverine.Redis.Tests DeadLetterQueueTests 6 green, which is where these fixes overlap the
GH-4559 rewrite of disabled_dead_letter_queue_should_not_create_dead_letter_stream (#4579).


Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

Co-authored-by: Jorge L. Torres M <tmorejon.jl@gmail.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.

Compliance tests assert Wolverine's configuration objects, not the broker's actual state

1 participant