BrighterCommand / BrighterCommand/Brighter
[Feat] Test Generator: Brighter-provided Reject/DLQ/requeue-with-delay flows as universal transport needs Generator Tests
- Dominant language
- C#
- Stars
- 2.5k
- Forks
- 296
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
## Problem
The messaging gateway generator's optional feature flags — `HasSupportToDelayedMessages` and `HasSupportToDeadLetterQueue` — are treated as *native*-capability switches, and are set to whatever makes the generated suite green. This is wrong on two counts:
1. **They gate behaviour Brighter provides universally.** Brighter falls back to its own scheduler for delayed send/requeue when a transport has no native delay, and provisions a dead-letter (and invalid-message) producer driven by the `Reject` flow when a transport has no native DLQ. `Reject(Message, MessageRejectionReason?)` is on `IAmAMessageConsumerSync`/`Async` for **every** transport. So these are universal conformance obligations, not opt-in capabilities.
2. **The flags are already mis-declared against the code.** Postgres has native delay (delay column in the insert) and native/provided DLQ, yet declares `HasSupportToDelayedMessages: false` and `HasSupportToDeadLetterQueue: false`. AWS SQS has native `DelaySeconds` (≤15 min) and declares delay `false`. RocketMQ is the only config declaring delay `true`, so the delayed-delivery test runs for exactly one transport.
The root cause is timing: **the Reject→DLQ / invalid-channel / requeue-via-producer / requeue-via-scheduler features post-date the generator.** Because they couldn't be expressed as templates, contributors hand-wrote them per transport instead. The result is broad but inconsistent duplication.
## Evidence: the same tests, hand-written per transport
Inventory of hand-written (non-generated) coverage across transport test projects:
| Transport | Reject→DLQ | Reject→invalid | Requeue w/ delay | Plain requeue | Delayed send | Fallback ladder / no-channel |
|---|---|---|---|---|---|---|
| RMQ.Async | ✅ | — | ✅ (producer) | — | — | — |
| RMQ.Sync | ✅ | — | ✅ (producer) | ✅ | ✅ | — |
| Kafka | ✅ | ✅ | ✅ (producer + scheduler) | ✅ | — | ✅ (+ metadata stamping) |
| AWS / AWS.V4 | ✅ | ✅ | — | — | — | ✅ |
| Redis | ✅ | ✅ | ✅ (producer) | ✅ (zero-delay) | — | ✅ |
| PostgresSQL | ✅ | ✅ | ✅ (native SQL) | ✅ | — | ✅ |
| MSSQL | ✅ | ✅ | ✅ (producer) | ✅ | — | ✅ |
| RocketMQ | ✅ | ✅ | — | — | — | ✅ |
| MQTT | ✅ | ✅ | ✅ (producer) | — | — | ✅ |
| Gcp | — | — | — | — | — | — |
| Azure / AzureServiceBus | — | — | — | ✅ (deferred, ASB only) | ✅ (Azure scheduler only) | — |
Representative source files:
- `tests/Paramore.Brighter.Kafka.Tests/MessagingGateway/Reactor/When_kafka_consumer_requeues_with_delay_should_use_producer.cs` and `...should_use_scheduler.cs`
- `tests/Paramore.Brighter.Kafka.Tests/MessagingGateway/Reactor/When_rejecting_message_with_delivery_error_should_send_to_dlq.cs`, `...unacceptable_reason_should_send_to_invalid_channel.cs`, `...unacceptable_and_no_invalid_channel_should_fallback_to_dlq.cs`, `...no_channels_configured_should_acknowledge_and_log.cs`, `...should_include_metadata.cs`
- `tests/Paramore.Brighter.RMQ.Sync.Tests/MessagingGateway/Reactor/When_rejecting_a_message_to_a_dead_letter_queue.cs`
- Equivalents in AWS, Redis, Postgres, MSSQL, RocketMQ, MQTT (consistent naming convention).
Reject→DLQ and the fallback ladder are covered on essentially every modern transport; Reject→invalid on most (absent on both RMQ variants); requeue-with-delay on six. **GCP has none of it. Delayed send is barely covered anywhere.** The commitment is real and near-universal — it just isn't enforced by the generator, and the gaps are exactly where a template would close them.
## Proposed work
1. **Inventory** the existing hand-written tests in these areas (table above is the starting point) and distil them into a canonical set of behaviours. Reconcile naming — the hand-written tests already share a convention (`...delivery_error_should_send_to_dlq`, `...unacceptable_and_no_invalid_channel_should_fallback_to_dlq`, `...no_channels_configured_...`).
2. **Design the effective test set** the generator should own, covering:
- Requeue with delay — via producer, and via scheduler fallback
- Reject → DLQ (Brighter-provisioned)
- Reject → invalid/unacceptable channel
- Fallback ladder (unacceptable → invalid, else → DLQ; delivery-error → DLQ)
- No channels configured (acknowledge/delete + log, per transport semantics)
- Rejection metadata stamping
- Delayed send / `SendWithDelay`
3. **Implement as templates for all transports**, ungated — enforcing that this is a universal Brighter commitment rather than a per-transport opt-in. This retires `HasSupportToDelayedMessages` and `HasSupportToDeadLetterQueue` as opt-in gates.
4. **Extend the provider interface.** These tests need what the current `IAmAMessageGateway{Reactor,Proactor}Provider` doesn't expose: a **consumer** configured with dead-letter and invalid-message routing keys. The InMemoryScheduler acts as a **scheduler** wired to the producer for these tests, to avoid needing to test across both the Gateway and the Scheduler.
5. If we still want to assert *native* behavior specifically (SQS redrive, RMQ DLX, native delay with no scheduler wired), reintroduce those as distinct `HasNative...` flags gating a small number of native-only tests — a genuinely different property from "Brighter provides this."
## Related defects to fix in the same pass
- **Broken `requeuing_with_delay` template.** `Templates/MessagingGateway/{Reactor,Proactor}/When_requeuing_a_failed_message_with_delay_should_receive_message_again` calls `Requeue(received)` with no `timeout` argument (`IAmAChannelSync.Requeue(Message, TimeSpan?)`), so it never exercises delayed requeue — it is the plain requeue test plus a `Thread.Sleep`, and weaker (it lacks the plain test's receive-retry loop). Its filename also matches both `requeuing` and `with_delay`, so it is currently gated on both flags.
- Correct the mis-declared configs (Postgres, SQS) as part of retiring/repurposing the flags.
## Notes
Surfaced while writing `docs/factories/tests`. Sibling generator defects: #4238 (single `Outbox` async-only), #4239 (`CollectionName` ignored by sync outbox templates). This one is larger and design-led — likely wants an ADR for the provider-interface change before implementation.
Contributor guide
Assessment
This issue has not been assessed yet.