Add MessageGateway test generator

[lillo42]

Overview

Extends the \Paramore.Brighter.Test.Generator` tool to automatically generate standardized tests for messaging gateway implementations (RabbitMQ, AWS SNS/SQS, Azure Service Bus, etc.), reducing code duplication and
ensuring consistent test coverage across all gateway implementations.

What's Changed

Configuration System

• Added MessagingGatewayConfiguration class with support for:
  • Test class naming and categorization
  • Custom message factories and assertion helpers
  • Gateway-specific publication/subscription configuration
• Async operation timing controls
• Extended TestConfiguration with MessagingGateway and MessagingGatewaies properties
• Added comprehensive XML documentation for all configuration classes

Generator Implementation

• New MessagingGatewayGenerator supporting both:
  • Reactor pattern (synchronous operations)
  • Proactor pattern (asynchronous operations)
• Follows the same architecture as the existing OutboxGenerator for consistency
• Generates provider interfaces and test implementations from Liquid templates

Message Factory & Assertions

• Refactored message creation to use MessageConfiguration for complete control over test message properties
• New IAmAMessageFactory interface and DefaultMessageFactory implementation
• New IAmAMessageAssertion interface and DefaultMessageAssertion implementation
• Factories now track created messages via CreatedMessages property

Templates

• MessagingGateway/Proactor/ - Async test templates
• MessagingGateway/Reactor/ - Sync test templates
• Provider interfaces and assertion implementations

Documentation

• Added ADR 0037 documenting the design decisions
• Added comprehensive XML documentation to all configuration classes and templates

Benefits

• ✅ Eliminates code duplication across gateway implementations
• ✅ Ensures consistent test coverage for all messaging gateways
• ✅ Reduces maintenance burden when test patterns change
• ✅ Simplifies adding new gateway implementations with complete test coverage

[claude]

Pull Request Review: Add MessageGateway Test Generator

Summary

This PR introduces a test generator for messaging gateway implementations, extending the existing test generation infrastructure to reduce code duplication across different messaging providers (RabbitMQ, AWS SNS/SQS, Azure Service Bus, etc.). The implementation follows the established pattern from the Outbox generator and includes comprehensive documentation via ADR 0037.

Positive Aspects ✅

Architecture & Design

• Excellent ADR documentation: ADR 0037 is comprehensive, well-structured, and clearly documents the context, decision, and consequences
• Consistent with existing patterns: Follows the same approach as OutboxGenerator, maintaining consistency in the codebase
• Good separation of concerns: Configuration, generation logic, and templates are properly separated
• Extensible design: Supports both single and multiple gateway configurations via MessagingGateway and MessagingGatewaies properties

Code Quality

• Proper licensing: All new files include the MIT license header as required
• Auto-generated markers: Generated files are properly marked with <auto-generated> comments
• XML documentation: Classes and public members include comprehensive XML documentation
• Type safety: Proper use of nullable reference types throughout

Issues & Concerns 🔴

1. Critical: Null Reference Issues in DefaultMessageFactory.cs

Location: tests/Paramore.Brighter.RMQ.Async.Tests/DefaultMessageFactory.cs:290-321 and template

The Create method has a major bug - it accepts MessageConfiguration? (nullable) but immediately dereferences it without null checking:

public Message Create(MessageConfiguration? configuration = null)
{
    var messageHeader = new MessageHeader(
        messageId: configuration.MessageId,  // ⚠️ NullReferenceException if configuration is null
        topic: configuration.Topic,
        // ... all other properties accessed without null check

Fix: Either:

1. Make parameter non-nullable: MessageConfiguration configuration with a default instance, OR
2. Add null coalescing: configuration ??= new MessageConfiguration(); at the start

Severity: HIGH - This will cause runtime exceptions

2. Code Style Violation: Inconsistent Spacing

Location: tools/Paramore.Brighter.Test.Generator/Templates/IAmAMessageFactory.cs.liquid:1526-1644

The MessageConfiguration class has inconsistent indentation. Properties use 2-space indents instead of the standard 4-space indents used elsewhere:

public class MessageConfiguration
{
    public Dictionary<string, object> Bag { get; set; } = new() { ... };

  public Baggage Baggage { get; set; } = new();  // ⚠️ 2 spaces instead of 4
  
  public ContentType ContentType { get; set; } = ...  // ⚠️ Inconsistent

Fix: Use consistent 4-space indentation for all properties

Severity: MEDIUM - Code style violation

3. Typo in Configuration Property Name

Location: tools/Paramore.Brighter.Test.Generator/Configuration/TestConfiguration.cs:877

Property name has a typo: MessagingGatewaies should be MessagingGateways (standard English plural)

public Dictionary<string, MessagingGatewayConfiguration>? MessagingGatewaies { get; set; }

Impact: This affects the public API of the configuration and should be corrected before release

Severity: MEDIUM - Breaking change required

4. Hard-coded Delay Values

Location: tests/Paramore.Brighter.RMQ.Async.Tests/MessagingGateway/Generated/Proactor/When_a_message_consumer_reads_multiple_messages_should_receive_all_messages.cs:590,607

The generated test has hard-coded delays:

• await Task.Delay(5000); (line 590)
• await Task.Delay(5000); (line 607)

While the configuration has DelayBetweenReceiveMessageInMilliseconds: 5000, these values should be configurable at the test configuration level, not hard-coded in the generated test. The template correctly uses the configuration value for the second delay (line 607), but the first delay at line 590 is hard-coded.

Severity: LOW - Test maintainability issue

5. Missing XML Documentation Comment

Location: tools/Paramore.Brighter.Test.Generator/Configuration/MessageConfiguration.cs (if it exists)

The MessageConfiguration class in the template lacks a class-level XML documentation comment. All public classes should have XML documentation per the project's documentation standards.

Severity: LOW - Documentation completeness

6. Commented-out Code in Generator

Location: tools/Paramore.Brighter.Test.Generator/Generators/MessagingGatewayGenerator.cs:941-947

// await GenerateAsync(
//     configuration,
//     Path.Combine("MessagingGateway", prefix, "Generated", "Reactor"),
//     Path.Combine("MessagingGateway", "Reactor"),
//     configuration.MessagingGateway,
//     filename => SkipTest(configuration.MessagingGateway, filename)
// );

Questions:

• Is Reactor (synchronous) support intentionally deferred?
• Should this commented code be removed or implemented?
• If deferred, should this be documented in the ADR?

Severity: LOW - Code cleanliness

7. Empty Skip Test Implementation

Location: tools/Paramore.Brighter.Test.Generator/Generators/MessagingGatewayGenerator.cs:1001-1004

private static bool SkipTest(MessagingGatewayConfiguration configuration, string fileName)
{
    return false;
}

This always returns false, unlike OutboxGenerator.SkipTest which has logic to skip transaction tests. Consider:

• Is this intentional or a placeholder?
• Should there be logic to skip certain tests based on gateway capabilities?

Severity: LOW - Potential missing functionality

Test Coverage Concerns ⚠️

Missing Test Scenarios

The PR generates only ONE test scenario: When_a_message_consumer_reads_multiple_messages_should_receive_all_messages. The ADR mentions "common scenarios" but only one is implemented.

Questions:

1. Are additional test scenarios planned (error handling, timeouts, connection failures, etc.)?
2. Is this intentional for the initial implementation?
3. Should the ADR clarify that only one scenario is currently implemented?

Recommendation: Consider adding at least:

• Test for single message send/receive
• Test for message acknowledgment/rejection
• Test for error handling scenarios

Security Considerations 🔒

No security issues identified. The generated tests use test doubles and don't expose credentials.

Performance Considerations ⚡

• 5-second delays in tests: The generated test has multiple 5-second delays which will slow down test execution. Consider if these can be reduced or made configurable per test environment.
• Test parallelization: Verify that generated tests can run in parallel without conflicts (unique routing keys/channels per test)

Recommendations 📋

Must Fix Before Merge (Breaking Issues)

1. ✅ Fix null reference exception in DefaultMessageFactory.Create()
2. ✅ Fix typo: MessagingGatewaies → MessagingGateways

Should Fix Before Merge (Quality Issues)

3. ✅ Fix inconsistent indentation in MessageConfiguration
4. ✅ Remove or document commented-out Reactor generation code
5. ✅ Add XML documentation for MessageConfiguration class
6. ✅ Make first delay in test template configurable (not hard-coded)

Consider for Future PRs

7. Add more test scenarios beyond multi-message read
8. Consider reducing test delay times for faster test execution
9. Implement skip logic for gateway-specific capabilities (if needed)
10. Add Reactor (sync) pattern support if planned

Testing Recommendations 🧪

Before merging, verify:

1. Run the generator against RMQ configuration and ensure tests compile
2. Run the generated tests to ensure they pass
3. Test with null MessageConfiguration parameter to verify the null handling fix
4. Verify generated code follows project conventions (run code formatter/linter)

Conclusion

This is a well-architected PR that follows established patterns and includes excellent documentation. The core design is solid, but there are several critical bugs that must be fixed before merge, particularly the null reference issue in DefaultMessageFactory. Once the must-fix items are addressed, this will be a valuable addition to the test infrastructure.

Overall Assessment: Approve with required changes

---

🤖 Generated with Claude Code

[claude]

Pull Request Review: Add MessageGateway Test Generator

Summary

This PR successfully extends the test generator tool to support messaging gateway tests, following the established pattern from the Outbox generator. The implementation is well-architected, consistent with the existing codebase, and includes comprehensive documentation via ADR 0037.

✅ Strengths

Architecture & Design

• Excellent consistency: Follows the same pattern as OutboxGenerator, making it easy for developers familiar with that pattern to understand
• Well-documented: ADR 0037 provides thorough context, rationale, and consequences
• Flexible configuration: Supports both single and multiple gateway configurations
• Good separation of concerns: Generator logic, templates, and provider implementations are cleanly separated

Code Quality

• Comprehensive XML documentation: All public APIs are well-documented
• Proper licensing: MIT license headers present in all files
• Auto-generated markers: Generated files clearly marked with <auto-generated> comments
• Proper async patterns: Correct use of Task, async/await, and CancellationToken

Testing Infrastructure

• Message configuration flexibility: MessageConfiguration class provides fine-grained control over test message properties
• Assertion abstraction: IAmAMessageAssertion allows custom validation logic
• Factory tracking: CreatedMessages property enables test verification

🔍 Issues & Concerns

1. Copy-paste error in log message (Minor)

Location: tools/Paramore.Brighter.Test.Generator/Generators/MessagingGatewayGenerator.cs:70

logger.LogInformation("Generating outbox test for {OutboxName}", key);

Should be:

logger.LogInformation("Generating messaging gateway test for {GatewayName}", key);

2. Commented-out code (Minor)

Location: MessagingGatewayGenerator.cs:50-56

Reactor pattern generation is commented out for single gateway configuration but enabled for multiple gateways (lines 87-93). This inconsistency suggests:

• Either Reactor support is incomplete and should be removed from the multiple gateway path too
• Or it should be uncommented for single gateway configuration

Recommendation: Remove commented code or add a TODO comment explaining why Reactor is disabled.

3. Typo in property name (Medium)

Location: tools/Paramore.Brighter.Test.Generator/Configuration/TestConfiguration.cs:48

public Dictionary<string, MessagingGatewayConfiguration>? MessagingGateways { get; set; }

Property is named MessagingGateways (with 's') but the ADR references it as MessagingGatewaies (line 41 of the ADR). While the code is correct, the ADR documentation has a typo.

4. Potential null reference issues (Low)

Location: Generated test template line 698

_subscription = _messageGatewayProvider.CreateSubscription(_publication.Topic!, ...)

Using null-forgiving operator (!) on _publication.Topic assumes it's always non-null. While this is likely safe given the factory method, consider adding a null check or assertion for robustness.

5. Hardcoded delay values (Medium - Performance)

Location: Generated test at lines 717 and 732

await Task.Delay(5000);  // Hardcoded 5-second delay

The test hardcodes a 5-second delay even though the configuration specifies DelayBetweenReceiveMessageInMilliseconds: 5000. If a developer changes the config to 1000ms (as shown in the ADR example on line 144), the hardcoded delays won't update.

Recommendation: Use the configuration value from _subscription or make it a template parameter.

6. Empty SkipTest implementation (Low)

Location: MessagingGatewayGenerator.cs:110-113

private static bool SkipTest(MessagingGatewayConfiguration configuration, string fileName)
{
    return false;
}

This stub exists in OutboxGenerator to skip transaction tests when not supported. For messaging gateways, consider:

• Are there test scenarios that some gateways don't support?
• If not, document why this always returns false (future extensibility)

7. Missing validation (Low)

Location: MessagingGatewayConfiguration.cs

The configuration properties Publication and Subscription are marked as string.Empty defaults but are required. Consider:

• Adding validation in the generator to fail fast with clear error messages
• Making them nullable and checking for null explicitly

💡 Suggestions for Improvement

1. Test Coverage

The PR only includes one generated test: When_a_message_consumer_reads_multiple_messages_should_receive_all_messages. Consider:

• Are there more test templates planned?
• Should there be tests for error scenarios (connection failures, malformed messages)?
• Single message send/receive tests?

2. Partition Key Handling

Location: Line 707-710 of generated test

_messageFactory.Create(new MessageConfiguration{Topic = _publication.Topic, PartitionKey = null}),

The partition key is explicitly set to null. This seems intentional but isn't documented. Consider:

• Adding a comment explaining why
• Or making it configurable per gateway

3. Resource Cleanup Ordering

Location: RmqMessageGatewayProvider.CleanUpAsync:24-35

Channel is disposed synchronously (channel.Dispose()) after async purge, while producer uses DisposeAsync(). Consider making channel disposal consistent by implementing IAsyncDisposable on channels or documenting why synchronous disposal is appropriate.

🔒 Security Considerations

No security concerns identified. The generated tests use localhost RabbitMQ connections with guest credentials, which is appropriate for test environments.

⚡ Performance Considerations

Test execution time: The hardcoded 5-second delays mean each 4-message test takes ~20+ seconds. For large test suites, this could be significant. Consider:

• Making delays configurable per environment (shorter for CI)
• Using polling with timeout instead of fixed delays
• Documenting expected test duration

📋 Documentation & Maintainability

Excellent documentation overall. The ADR is thorough and includes:

• Clear context and rationale
• Consequences (positive and negative)
• Implementation examples
• Migration path

Minor improvement: Add a section in the ADR about how to add new test scenarios in the future.

🎯 Recommendations

Must Fix (Before Merge)

1. Fix the log message copy-paste error (line 70 of MessagingGatewayGenerator.cs)

Should Fix (Before or Shortly After Merge)

2. Remove or document commented Reactor code
3. Fix typo in ADR (MessagingGatewaies → MessagingGateways)
4. Use configuration value instead of hardcoded delays in templates

Nice to Have (Future Work)

5. Add validation for required configuration properties
6. Document or expand test scenario coverage
7. Consider performance impact of delays and optimize if needed

✅ Final Assessment

Recommendation: APPROVE with minor changes

This is a high-quality PR that:

• Follows established architectural patterns
• Includes excellent documentation
• Provides real value by reducing test duplication
• Has comprehensive XML documentation
• Includes working example (RabbitMQ)

The issues identified are mostly minor and can be addressed in follow-up commits or as part of this PR. The core architecture is sound and ready for use.

Estimated risk level: Low
Test coverage impact: Positive (enables consistent coverage across gateways)
Maintenance burden: Low (follows established patterns)

---

Great work @lillo42! This is a valuable addition to the test infrastructure. 🎉

[iancooper]

I just want to say how excited I am for this.

[iancooper]

I can think of a few other points, but let's start with these.

[iancooper]

I note that I have had more success when running the Dispatcher using a handler that takes the FakeTimeProvider to advance the time, rather than using Task Delay, when timing is being used to trigger a behaviour. It's possible a few tests could benefit from this

[iancooper]

Where to start when you want to add a new feature to our MessagingGateways as well. Much harder to iterate on in-memory and then roll out. We need to document the recommended loop

[iancooper]

This seems a little bit unnecessary. What forces this?

[lillo42]

It's part of the IAsyncLifeTime

[iancooper]

Why is this an interface? Not sure I understand the thinking

[lillo42]

We need to assert the received message against the provided message. In some message gateways like RabbitMQ, the MessageId will be different on message Requeue, so I want to use this interface to cover this cases

[claude]

Pull Request Review: Add MessageGateway Test Generator

Summary

This PR extends the Paramore.Brighter.Test.Generator tool to automatically generate standardized tests for messaging gateway implementations. The implementation follows the established pattern from OutboxGenerator and includes comprehensive ADR documentation. Overall, this is a well-architected solution that will significantly reduce code duplication and improve consistency across gateway implementations.

✅ Strengths

1. Excellent Architecture & Design

   • Follows existing patterns from OutboxGenerator for consistency
   • Clean separation of concerns with provider interfaces
   • Comprehensive ADR (0037) documenting design decisions and trade-offs
   • Supports both Reactor (sync) and Proactor (async) patterns

2. Code Quality

   • Proper MIT licensing on all files
   • Auto-generated file markers (<auto-generated> comments)
   • Comprehensive XML documentation on configuration classes
   • Clean, readable code structure

3. Configurability & Flexibility

   • Feature flags (HasSupportToPublishConfirmation, HasSupportToDelayedMessages, etc.) allow gateways to opt-out of unsupported features
   • Supports both single gateway (MessagingGateway) and multiple gateways (MessagingGateways) configurations
   • Liquid templates enable customization without code changes
   • Configuration inheritance from parent TestConfiguration

4. Test Coverage

   • Generated 18 test files for RabbitMQ (9 Proactor + 9 Reactor)
   • Comprehensive test scenarios: message publishing, receiving, requeuing, dead-letter queue, activity tracing, etc.
   • Provider pattern allows gateway-specific implementations while maintaining consistent test structure

🔍 Issues & Concerns

Critical Issues

1. Naming Inconsistency: MessageFactory vs MessageBuilder (Lines: MessagingGatewayConfiguration.cs:44, ADR mentions both)

• The configuration class uses MessageBuilder (line 44)
• The ADR documentation refers to MessageFactory (line 30, 136)
• The actual implementation uses IAmAMessageBuilder and DefaultMessageBuilder

Impact: This creates confusion in documentation and may cause issues when developers read the ADR vs implement the configuration.

Recommendation:

// Update ADR to consistently use "MessageBuilder" terminology
// OR rename MessageBuilder to MessageFactory throughout the codebase

2. Missing XML Documentation on Configuration Properties (MessagingGatewayConfiguration.cs:78-86)
The boolean feature flags lack XML documentation:

public bool HasSupportToPublishConfirmation { get; set; }
public bool HasSupportToDelayedMessages { get; set; }
public bool HasSupportToPartitionKey { get; set; }
public bool HasSupportToDeadLetterQueue { get; set; }
public bool HasSupportToValidateBrokerExistence { get; set; }

Impact: Developers won't understand what these flags control without reading the generator code.

Recommendation: Add XML documentation explaining each flag's purpose and default behavior.

High Priority Issues

3. Hardcoded Delay Value (When_posting_a_message_via_the_messaging_gateway_should_be_received.cs:61)

await Task.Delay(5000);

This 5-second delay is hardcoded in generated tests, despite DelayBetweenReceiveMessageInMilliseconds being configurable.

Impact: Tests will always wait 5 seconds regardless of configuration, making tests slower than necessary.

Recommendation: Use the configured delay value from MessagingGatewayConfiguration.DelayBetweenReceiveMessageInMilliseconds in the template.

4. Incomplete Dead Letter Queue Implementation (RmqMessageGatewayProvider.cs:152-163)

public Task<Message> GetMessageFromDeadLetterQueueAsync(...)
{
    throw new NotImplementedException();
}

Impact: Tests requiring DLQ validation will fail at runtime with NotImplementedException.

Recommendation: Either implement these methods or conditionally skip DLQ tests when the provider doesn't support them.

5. Missing Default Values in Configuration
MessagingGatewayConfiguration properties like MessageGatewayProvider, Category, and delay settings have no defaults or validation.

Impact: Null reference exceptions if configuration is incomplete.

Recommendation: Add validation in MessagingGatewayGenerator.GenerateAsync() or provide sensible defaults.

Medium Priority Issues

6. ADR Date Mismatch
The ADR is dated 2026-01-29 but the PR was created in February 2026. Minor issue but dates should be accurate.

7. Timestamp Comparison Precision Loss (DefaultMessageAssertion.cs:57)

Xunit.Assert.Equal(expected.Header.TimeStamp.ToString("yyyy-MM-ddTHH:mm:ss"), 
                   actual.Header.TimeStamp.ToString("yyyy-MM-ddTHH:mm:ss"));

Impact: Loses millisecond precision which might be relevant for some scenarios.

Recommendation: Consider whether millisecond precision matters for your use case. If not, add a comment explaining the precision loss is intentional.

8. Generator Log Message Inconsistency (MessagingGatewayGenerator.cs:70)

logger.LogInformation("Generating outbox test for {OutboxName}", key);

Should say "messaging gateway test" not "outbox test".

🎯 Recommendations

Documentation

1. Fix the MessageFactory/MessageBuilder naming inconsistency throughout the PR
2. Add XML documentation to all boolean feature flags
3. Update ADR with accurate dates
4. Consider adding a README in the templates folder explaining the Liquid template structure

Configuration

5. Add validation for required configuration properties
6. Consider adding default values for optional settings
7. Document the relationship between configuration inheritance and override behavior

Testing

8. Use the configured delay value instead of hardcoded delays
9. Implement or properly handle the DLQ methods
10. Consider adding a test configuration validator to catch misconfiguration early

Code Quality

11. Fix the log message in MessagingGatewayGenerator.cs:70
12. Consider adding integration tests for the generator itself
13. Document the expected provider interface implementation patterns

📊 Performance Considerations

The 5-second delay in tests could make test suites slow. Consider:

• Making delays configurable per test scenario
• Using polling with timeout instead of fixed delays
• Adding configuration to disable delays in CI environments

🔒 Security Considerations

No security concerns identified. The generator creates test code only and doesn't handle sensitive data.

📝 Test Coverage Assessment

Generated tests cover the main scenarios well:

• ✅ Basic message publishing/receiving
• ✅ Multiple message handling
• ✅ Requeuing and error scenarios
• ✅ Activity/trace context propagation
• ✅ Infrastructure validation
• ⚠️ Dead letter queue (implementation incomplete)
• ⚠️ Partition keys (tests exist but need provider support)

🎉 Conclusion

This is a well-designed feature that will provide significant value to the Brighter project. The architecture is solid, the code is clean, and the documentation is comprehensive. The issues identified are mostly minor and can be addressed with targeted fixes.

Recommendation: Approve with minor changes

The critical issues around naming consistency and missing XML documentation should be addressed before merge, but they don't block the overall architecture. The incomplete DLQ implementation is acceptable if documented as future work.

Great work! This will make gateway implementation testing much more consistent and maintainable.

---

Review generated by Claude Code 🤖

[claude]

test comment

[claude]

Code Review: Add MessageGateway Test Generator Part 1 of 2

Thanks for this PR - the concept of a test generator for messaging gateway implementations is a great idea that will ensure consistent coverage across providers. Here is my detailed feedback.

Bugs

Bug 1: async void Dispose() in Reactor Test Templates

The Reactor test templates (and generated files) have async void Dispose() but the method body is entirely synchronous. async void methods are a well-known C# anti-pattern because unhandled exceptions become unobserved and can crash the process. Fix: Remove the async keyword (keep public void Dispose()).

Affected templates:

• Templates/MessagingGateway/Reactor/When_a_message_consumer_reads_multiple_messages_should_receive_all_messages.cs.liquid
• Templates/MessagingGateway/Reactor/When_confirming_posting_a_message_should_receive_publish_confirmation.cs.liquid
• And their generated counterparts under MessagingGateway/Generated/Reactor/

Bug 2: Class Name Mismatch

The file is named MessagingGatewayGenerator.cs but the class inside is MessageGatewayGenerator (missing ing). Rename the class to MessagingGatewayGenerator.

Bug 3: Copy-Paste Error in Log Message

In MessagingGatewayGenerator.cs the foreach loop for multiple gateways contains a copy-paste from OutboxGenerator: logger.LogInformation says Generating outbox test for OutboxName but should say Generating messaging gateway test for GatewayName.

[claude]

Code Review: Add MessageGateway Test Generator Part 2 of 2

Design Issues

Issue 4: Vacuous Assert.True(true) in Exception Tests

The should throw exception tests use a pattern that always passes regardless of whether any exception is thrown. The catch block uses Assert.True(true) which provides zero assertion value. This appears in 6 templates/generated files (both Proactor and Reactor variants for assume_channel, validate_channel, no_broker_created). Use Assert.ThrowsAnyAsync or Record.ExceptionAsync instead, or omit the catch body.

Issue 5: GetOrCreateRoutingKey and GetOrCreateChannelName Ignore the testName Parameter

In RmqMessageGatewayProvider, both methods ignore testName entirely and always return fresh random UUIDs. The contract implied by GetOrCreate and the CallerMemberName attribute is that calling the method multiple times with the same test name returns the same value. A test that calls GetOrCreateRoutingKey() twice would get mismatched routing keys. Either implement a dictionary-based cache keyed on testName, or rename to CreateRoutingKey / CreateChannelName.

Issue 6: GetMessageFromDeadLetterQueue is Unimplemented

Both GetMessageFromDeadLetterQueueAsync and GetMessageFromDeadLetterQueue in RmqMessageGatewayProvider throw NotImplementedException. Either implement them or set HasSupportToDeadLetterQueue = false in test-configuration.json to skip those tests.

Issue 7: test-configuration.json Missing DestinationFolder and Category

Without DestinationFolder, the generator falls back to Directory.GetCurrentDirectory() at runtime. Without Category, generated tests will not get a Trait Category RabbitMQ attribute, breaking CI filtering. The ADR example configuration (line 147) shows category RabbitMQ but it is absent from the actual config file.

Issue 8: Generated Files Committed Without Documenting the Intent

The PR commits 20+ auto-generated files alongside the templates. This creates a divergence risk. Please clarify in the ADR or a README in the Generated folder: are these files intentionally committed as a reference snapshot, or should they be gitignored and regenerated as part of the build?

Minor Nits

9. The generated interfaces declare non-nullable testName = null! while the concrete implementation correctly uses string? testName = null. Prefer the nullable form in the interface templates.

10. ADR line 47: MessagingGatewaies should be MessagingGateways.

11. In IAmAMessageGatewayReactorProvider, the CreateChannel doc comment says An asynchronous channel for receiving messages, a copy-paste error from the Proactor template.

12. The five HasSupportTo boolean properties in MessagingGatewayConfiguration lack XML doc summaries.

Overall

This is a well-structured feature that follows the established OutboxGenerator design and the ADR is thorough. The main items to address before merging:

1. Fix the async void Dispose() bug in the Reactor templates and regenerate affected files
2. Rename MessageGatewayGenerator to MessagingGatewayGenerator
3. Fix the log message copy-paste error
4. Address the GetOrCreate contract issue in the provider or rename the methods
5. Add Category and DestinationFolder to test-configuration.json
6. Implement or explicitly skip the dead letter queue tests in the RMQ provider

[claude]

Review: Add MessageGateway test generator (#3996)

Thanks for this — extending the test generator to messaging gateways is a great direction, and the bulk of the 1,131 files are mechanically generated tests, which I treated as outputs rather than reviewing line-by-line. I focused on the hand-written parts: the generator/config code under tools/, the three production src/ changes, the templates, and the CI/infra changes. Overall it looks solid and consistent with the existing OutboxGenerator. A few things worth addressing before merge.

🐞 Potential bugs / correctness

1. src/.../AzureServiceBus/AzureServiceBusMesssageCreator.cs — a behavioral change is buried in pure formatting churn. ~95% of this file's diff is line-rewrapping (no behavior change), but one real change hides in it:

   - headers.Bag.Add(property.Key, property.Value);
   + headers.Bag[property.Key] = property.Value;

   Dictionary.Add throws on a duplicate key; the indexer silently overwrites. Since LockTokenHeaderBagKey and SequenceNumberBagKey are added to Bag before this loop, an ApplicationProperties collision previously threw and now silently overwrites. This is plausibly a bug fix, but it's undocumented and easy to miss among the reformatting. Per CLAUDE.md ("Change Scope: Do NOT … make changes beyond what was explicitly requested — no additional 'improvements'"), the wholesale reformatting of this file is out of scope and makes the meaningful change hard to spot. Please split the formatting out (or revert it) and keep the behavioral change as its own clearly-described commit.

2. MessageGatewayGenerator.cs — wrong log message (copy-paste from OutboxGenerator). In the multi-gateway branch:

   logger.LogInformation("Generating outbox test for {OutboxName}", key);

   This is the messaging-gateway generator; the message and {OutboxName} placeholder should refer to the gateway, not the outbox.

3. Filename vs. class name mismatch. The file is Generators/MessagingGatewayGenerator.cs but the class is MessageGatewayGenerator (and the PR title/description say MessagingGatewayGenerator). It compiles and Program.cs references the right name, but the inconsistency is confusing — recommend renaming the class to MessagingGatewayGenerator to match the file and the rest of the codebase's naming.

🔁 Consistency

4. OriginalMessageId on requeue diverges from the established RMQ pattern. The new MsSql/Postgres code does:

   if (!message.Header.Bag.ContainsKey(Message.OriginalMessageIdHeaderName))
       message.Header.Bag[Message.OriginalMessageIdHeaderName] = message.Header.MessageId.ToString();

   whereas RmqMessagePublisher.AddOriginalMessageIdOnRepublish (a) uses a case-insensitive key check (Bag.Any(h => h.Key.Equals(..., CurrentCultureIgnoreCase))) and (b) writes to the outgoing headers rather than mutating the in-memory message.Header.Bag. The new code uses a case-sensitive ContainsKey and mutates the live Message. The behavior is reasonable and the logic is duplicated identically across both sync/async paths in each consumer (good), but consider aligning the key comparison with RMQ to avoid a subtle "two original-id headers with different casing" situation, and confirm mutating the passed-in Message is acceptable in the requeue flow.

⚙️ CI / infra (looks intentional, but please confirm)

5. KAFKA_AUTO_CREATE_TOPICS_ENABLE flipped true → false (and delete-topic false → true) in ci.yml. This is a meaningful change to existing Kafka CI behavior — any current Kafka test relying on auto-creation will now need explicit topic creation. Presumably the generated tests handle this, but worth double-checking the existing Kafka suite still passes.

6. RocketMQ dashboard port mapping "8082:8082" (was "8080:8080") with platform: linux/amd64 commented out. The dashboard image listens on 8080 inside the container, so 8082:8082 likely won't reach the UI (probably meant 8082:8080). Dev-only/optional, so low impact, but looks like a mistake.

7. docker-compose-localstack.yaml and docker-compose-kafka.yaml: you removed the obsolete version: '3' from kafka but the new localstack file adds it back. Minor inconsistency.

🧹 Minor

8. Dead/no-op first generation call. GenerateAsync(configuration, "MessagingGateway", "MessagingGateway", …) enumerates Templates/MessagingGateway/*.liquid (top-directory only), which is empty — all templates live in Reactor//Proactor/. It generates nothing. This mirrors the pre-existing OutboxGenerator pattern (so not new), but both are redundant calls that could be dropped for clarity.

9. Config rename MessageFactory → MessageBuilder is a breaking schema change. Any existing test-configuration.json using MessageFactory will silently fall back to the default DefaultMessageBuilder after this change (note the PR description still mentions IAmAMessageFactory/DefaultMessageFactory, which no longer match the code). Confirm all in-repo config files were updated, and consider calling this out in the ADR/release notes since it's a user-facing config break for the tool.

10. SkipTest relies on substring matching of template paths (fileName.Contains("requeuing"), "dead_letter_queue", etc.). It works for the current template names, but it's fragile — a future template rename could silently disable or enable a test. A short comment near SkipTest documenting the naming contract would help.

✅ Strengths

• Generator follows the existing OutboxGenerator/BaseGenerator architecture closely — easy to follow.
• Good XML doc coverage on the new config and generator classes (consistent with repo standards).
• MessagingGatewayConfiguration defaults (e.g. HasSupportToValidateInfrastructure = true, timeouts) are sensible.
• Feature-flag gating of unsupported scenarios (delay, partition key, DLQ, confirmations) is a clean way to keep the matrix correct per gateway.
• nullable enabled on the tool project and multi-targeting via <TargetFrameworks> is a nice tidy-up.

Nothing here is blocking on its own, but I'd prioritize #1 (separate the ASB reformatting from the behavioral change) and #2/#3 (the log message and class name) before merge. Nice work overall.

🤖 Automated review — verify findings before acting.

[claude]

Code Review — Add MessageGateway test generator (#3996)

Thanks for this — extending the generator to messaging gateways is a nice consistency win, and the implementation faithfully mirrors the established OutboxGenerator shape. The XML docs added across the configuration/generator classes are a real improvement. Most of the 1,131 files are generated output / Liquid templates; my review focuses on the hand-written code under tools/Paramore.Brighter.Test.Generator/.

🐞 Bugs / correctness

1. Dead partition_key skip branch — capability with no test. MessageGatewayGenerator.SkipTest skips templates whose name contains partition_key, but no template under Templates/MessagingGateway/{Reactor,Proactor}/ matches that token. Meanwhile HasSupportToPartitionKey is set across many configs (and true for AWS FIFO entries). Net effect: the flag does nothing and no partition-key test is ever generated. Either add the missing template or drop the dead branch + flag so it doesn't read as covered when it isn't.

2. Copy-paste log message. In the multiple-gateways branch:

   logger.LogInformation("Generating outbox test for {OutboxName}", key);

   This is generating a messaging gateway test, not an outbox — the message and placeholder name are leftover from OutboxGenerator.

3. No-op first GenerateAsync call. Both the single- and multiple-gateway paths start with GenerateAsync(configuration, "MessagingGateway", "MessagingGateway", ...), but Templates/MessagingGateway/ has no top-level *.liquid files (only Reactor/ and Proactor/ subfolders). The call just logs Found 0 liquid files and does nothing. In OutboxGenerator the equivalent call is meaningful because Templates/Outbox/ does contain top-level templates. Harmless but confusing — worth removing.

🎨 Naming / consistency

4. Class name ≠ file name, and Message vs Messaging. The file is MessagingGatewayGenerator.cs but the class is MessageGatewayGenerator; similarly the config property is MessageGatewayProvider. Brighter's convention is Messaging (IAmAMessagingGateway, IAmAMessageConsumer). Recommend standardizing on MessagingGatewayGenerator / MessagingGatewayProvider so file and type names match.

5. ADR number collision. docs/adr/0037-add-messaging-gateway-generated-test.md reuses 0037, which already has four files. The latest sequential number is 0064, so this should likely be 0065.

6. Missing XML doc on OutboxConfiguration.CollectionName. The new Category got a doc comment but CollectionName did not, while its neighbors are all documented — inconsistent with the repo's documentation standards.

🧪 Test coverage

7. No tests for the new generator. tests/Paramore.Brighter.Test.Generator.Tests/ already covers OutboxGenerator (incl. parent-default application and skip behavior) and SharedGenerator, but there are no tests for MessageGatewayGenerator — its SkipTest matrix (exactly the kind of substring logic that produced bug Support for multiple Application Layer Protocols in Task Queues #1), the parent-default override, and the multiple-gateway prefix handling all go unverified. CLAUDE.md mandates TDD; equivalent coverage to the Outbox generator would have caught the dead partition_key branch.

⚠️ Behavioral note

8. MessageFactory → MessageBuilder rename is silently lossy. TestConfiguration/OutboxConfiguration renamed MessageFactory to MessageBuilder. Because System.Text.Json ignores unknown properties by default, any pre-existing test-configuration.json still using MessageFactory will silently fall back to the default rather than erroring. The in-repo configs are updated, so this is mainly a heads-up for downstream/local configs.

Minor

• SkipTest's substring matching (requeuing, with_delay, delayed_message) is name-fragile — future template renames can silently change which tests are emitted; the tests in Should Brighter move into its own repository #7 would lock this down.
• Mutating the input config (messagingGatewayConfiguration.Prefix = $".{prefix}") is a side effect carried over from OutboxGenerator — not new, just noting the smell.
• The generated activity-context test creates a TracerProvider (and parentActivity) it never disposes — a minor leak in the generated tests.

👍 What's good

• Clean reuse of BaseGenerator/SharedGenerator plumbing; feature-flag-driven skipping keeps per-gateway output minimal.
• The generated IAmAMessageGateway{Reactor,Proactor}Provider abstraction is a tidy seam for each transport to implement.
• Real adoption across RMQ, AWS, Kafka, Redis, Postgres, MSSQL, GCP, RocketMQ, etc.
• Nullable enabled on the tool project and net10.0 added to the target frameworks.

Overall a solid, well-structured change. Addressing #1, #2, and #7 would be the highest-value follow-ups.

_{🤖 Automated review by Claude Code. I could not run the build here (sandbox restrictions), so please confirm CI is green.}

[claude]

PR Review: Add MessageGateway test generator

Thanks for this — extending the test generator to messaging gateways is a solid step toward consistent, low-duplication coverage across transports, and following the existing OutboxGenerator shape makes it easy to follow. The XML docs are thorough and the per-gateway feature flags are a nice touch. A few findings below, grouped by area.

🐞 Bugs / correctness

1. Stray empty statement in OutboxGenerator.cs:73-74. A leftover ; on its own line:

   outboxConfiguration.Prefix = $".{prefix}";
       ;

   Harmless (compiles) but clearly an accidental artifact from the reformat — please remove.

2. partition_key skip branch is dead code. SkipTest checks fileName.Contains("partition_key") and HasSupportToPartitionKey exists in the config, but no template contains partition_key (confirmed across both Reactor/ and Proactor/ folders). So the flag and the branch currently do nothing. Either add the partition-key test template you intended, or drop the flag + branch until it exists — otherwise it reads as covered behavior that isn't there.

3. Copy-paste log placeholder in the multi-gateway loop (MessageGatewayGenerator.GenerateAsync):

   logger.LogInformation("Generating messaging gateway test for {OutboxName}", key);

   The structured-logging property is named OutboxName for a messaging gateway. Rename to e.g. {GatewayName}.

🧹 Code quality / consistency

4. Naming: MessageGatewayGenerator vs Messaging. The new class/file is MessageGatewayGenerator (no "ing"), while everything around it is Messaging — MessagingGatewayConfiguration, TestConfiguration.MessagingGateway, the templates, and even the ADR itself refers to MessagingGatewayGenerator. Recommend renaming the class/file to MessagingGatewayGenerator so the codebase and ADR agree.

5. SkipTest matches against the full file path with a case-sensitive Contains. The outbox version uses StringComparison.InvariantCultureIgnoreCase; the gateway version uses the default ordinal/case-sensitive Contains. The lowercase substrings happen to match the lowercase filenames, so it works today, but (a) it's inconsistent with the sibling method and (b) matching the whole path (not just the file name) means a directory component could one day match accidentally. Consider matching on Path.GetFileName(fileName) and being explicit about the comparison. Also note requeuing matches three templates including ...move_to_dead_letter_queue — intentional coupling of requeue→DLQ, but worth a comment so it isn't surprising.

⚠️ Required-config validation / hidden ordering

6. Required fields aren't validated and rely on generator ordering. Templates emit new {{ MessageBuilder }}(), new {{ MessageAssertion }}(), new {{ MessageGatewayProvider }}(), plus {{ Publication }}/{{ Subscription }}. MessageBuilder/MessageAssertion only get sensible defaults because SharedGenerator runs first in Program.cs and mutates the shared configuration instance that the gateway generator later reads — an implicit dependency that's easy to break. MessageGatewayProvider/Publication/Subscription have no defaults and no validation: omitting them silently generates uncompilable code like new (). Consider validating these up front and logging a clear error (the tool already logs+returns on other bad input).

🧪 Test coverage (the main gap)

7. No unit tests for MessageGatewayGenerator. OutboxGenerator, SharedGenerator, and Parser all have tests under tests/Paramore.Brighter.Test.Generator.Tests/, and the existing outbox tests were updated for the MessageFactory→MessageBuilder rename — but the new generator ships with none. Its SkipTest feature-gating (8 branches) and the parent-config fallback for MessageBuilder/Namespace/MessageAssertion, plus single-vs-MessagingGateways dictionary handling, are exactly the non-trivial logic that wants tests. Given the repository's mandatory TDD workflow (CLAUDE.md), please add coverage mirroring the existing OutboxGenerator tests — at minimum: feature flag → template skipped/kept, parent-namespace/builder fallback, and the prefix-from-dictionary-key path.

Performance / security

Nothing of concern — this is a build-time code generator doing sequential local file I/O, which is fine at this scale. The csproj change (TargetFramework→TargetFrameworks + Nullable=enable) looks correct and is a good fit for the new nullable-aware code.

Note on review scope

This PR is 1,135 files; the vast majority are generated .cs/.verified.cs test outputs. I focused on the generator source, configuration, templates, and wiring. I confirmed the generated output is wired to real hand-written providers (e.g. RedisMessageGatewayProvider) and that no .received.* approval-test artifacts were committed (good). I did not attempt to compile the full generated test set.

---

Overall: nice direction and consistent with the existing generator. The dead partition_key branch (#2), the missing generator unit tests (#7), and the naming inconsistency (#4) are the ones I'd most want addressed before merge.

🤖 Generated with Claude Code

[claude]

Code Review — PR #3996: Add MessageGateway test generator

Thanks for this — extending the test generator to messaging gateways is a nice way to drive consistency across transports, and the new code closely mirrors the established OutboxGenerator pattern. Most of the 1,135 files are generated output; this review focuses on the hand-written source, templates, and the production/CI changes bundled in.

What's good

• MessagingGatewayGenerator follows the existing OutboxGenerator/SharedGenerator architecture faithfully, including the SkipTest predicate and the GenerateAsync override that back-fills defaults from the parent config. Consistent and easy to follow.
• Thorough XML documentation on all new configuration and generator types, and a supporting ADR (0037).
• Genuine bug fix in PostgresMessageConsumer.Receive: timeOut.Value.Seconds -> timeOut.Value.TotalSeconds. .Seconds only returns the 0-59 component, so any timeout >= 1 minute was being truncated (e.g. a 90s timeout became 30s). The async path already used TotalSeconds, so this aligns them.
• Adding OriginalMessageId to the bag on requeue in MsSqlMessageConsumer / PostgresMessageConsumer brings them in line with the RMQ transports' existing convention.

Main concern: scope

Per CLAUDE.md ("Change Scope: do NOT make changes beyond what was explicitly requested"), this PR mixes a tooling feature with unrelated production behavior and CI changes that would be safer reviewed/bisected on their own:

1. Production runtime changes — the OriginalMessageId-on-requeue additions (MsSql x2, Postgres x2) and the Postgres timeout fix change consumer behavior at runtime. These are reasonable, but they're invisible in a PR titled "test generator" and aren't covered by hand-written unit tests in the diff (only by the generated transport tests, which require live brokers).
2. AzureServiceBusMessageCreator is a rename of AzureServiceBusMesssageCreator (R052) fixing the triple-s typo — good — but note the backing field in AzureServiceBusConsumer is still _azureServiceBusMesssageCreator (typo retained). Cosmetic, but worth finishing the rename.
3. CI changes (ci.yml): the Kafka env flip stands out — KAFKA_AUTO_CREATE_TOPICS_ENABLE: true -> "false" and KAFKA_DELETE_TOPIC_ENABLE: "false" -> "true". Disabling topic auto-creation is a meaningful semantic change; any Kafka test relying on implicit topic creation will now fail. Also bundled: RMQ job split into async/sync, GCP pinned to net10.0 only (9.0.x dropped) with extra excluded categories and a longer timeout. None of these relate to the generator — please call them out explicitly or split them.

Test coverage gap

The new MessagingGatewayGenerator — the core of this PR — has no direct unit tests. OutboxGenerator and SharedGenerator both have dedicated tests (and several were updated here for the MessageFactory->MessageBuilder rename), so the precedent and harness already exist. The untested logic that most warrants it:

• SkipTest(...) — each feature flag -> skipped-template mapping (publish confirmation, delayed/with_delay, dead-letter, broker existence, requeue, validate/assume channel).
• The single-vs-multiple-gateway prefix handling and the default back-fill override.

Given the repo's mandatory TDD guidance, this is the thing I'd most want addressed before merge.

Smaller points

• Brittle skip matching: SkipTest keys off substrings of the template file name (Contains("with_delay"), "requeuing", "dead_letter_queue", etc.). Renaming a template silently breaks the skip with no compile-time signal. A small metadata/manifest per template, or named constants, would be more robust.
• Single vs. multiple Prefix inconsistency (inherited from OutboxGenerator): in the multiple-gateway path the code does config.Prefix = $".{prefix}", auto-prepending the dot used by the template namespace ...MessagingGateway{{ Prefix }}.Reactor; in the single path the prefix is used verbatim, so the author must include the leading . themselves. Easy to misconfigure — worth documenting on the Prefix property, or normalizing both paths.
• Test flakiness risk: the "should_be_received" templates do a single _channel.Receive(...) (default 300 ms) with no poll/retry, then assert MessageType != MT_NONE. Against real brokers in CI that one-shot 300 ms window is prone to intermittent failures, and the assertion gives a poor diagnostic on timeout. DelayBetweenReceiveMessageInMilliseconds exists but isn't applied in that template. Consider a bounded retry loop.
• PR description drift: the description references IAmAMessageFactory/DefaultMessageFactory and MessagingGatewaies, but the code ships IAmAMessageBuilder/DefaultMessageBuilder and MessagingGateways. Please refresh the description so reviewers aren't chasing the wrong names.
• Pre-existing, carried over (not introduced here, but flagging since the file moved): GetSource/GetCloudEventsDataSchema in the ASB creator call new Uri(value) on inbound message properties without TryCreate, so a malformed source/dataschema header would throw UriFormatException during MapToBrighterMessage — unlike the time path which uses TryParse. Worth hardening at some point.

Summary

The generator itself is solid and well-aligned with existing patterns. The blockers I'd want resolved before merge: (1) add unit tests for MessagingGatewayGenerator/SkipTest, and (2) separate or at minimum explicitly justify the bundled production-consumer and CI changes — especially the Kafka auto-create-topics flip. The rest are polish.

Automated review — please verify against your own judgment.

[claude]

Review: Add MessageGateway test generator (#3996)

Thanks for this — extending the Test.Generator to cover messaging gateways is a genuinely valuable direction, and the new code closely mirrors the existing OutboxGenerator so it slots in cleanly. The XML docs are thorough, the feature-flag skip model is a nice touch, and ADR 0037 documents the rationale. I reviewed the generator tool source, the production src/ changes, the ADR, and a sample of the templates. Feedback below, roughly in priority order.

Significant

1. PR scope and size (1136 files, +49k/-18k). This bundles three quite different things: (a) the generator tool, (b) thousands of generated/migrated tests, and (c) behavioral changes to production code in src/. Per CLAUDE.md ("Change Scope" + Tidy First), mixing structural/behavioral/generated changes in one PR makes it very hard to review safely and to bisect later. Consider splitting at least the production src/ fixes (below) into their own small PRs so they can be reviewed and reverted independently of the test-generation churn.

2. No tests for MessagingGatewayGenerator. The generator test project has 8 tests for OutboxGenerator (prefix/namespace/skip behaviors) but zero for MessagingGatewayGenerator. Its new SkipTest has seven independent skip branches driven by substring matching, and the multi-gateway prefix logic (Prefix = $".{prefix}") is non-trivial — all currently unverified. Given the repo's mandatory TDD policy this is the gap I'd most want closed: mirror the existing OutboxGenerator tests for each HasSupportTo* flag and for the single-vs-multiple gateway paths.

3. MsSqlOutbox.CreateSqlParameter forces DbType.DateTime, ignoring the caller's dbType. In the (string, DbType, object?) overload, for any DateTimeOffset value the code returns new SqlParameter { ..., Value = dateTimeOffset.ToUniversalTime().DateTime, DbType = DbType.DateTime } — discarding the dbType argument and hardcoding DbType.DateTime. SQL datetime has ~3.33ms resolution and a min of 1753-01-01, whereas the column may be datetime2/datetimeoffset. That's a silent precision (and potential range) downgrade on timestamps the outbox uses for outstanding/dispatched age filtering. Is forcing DateTime intentional, or should it honour the requested dbType (e.g. DateTime2)? The round-trip itself looks consistent (GetTimeStamp re-specifies DateTimeKind.Utc); the concern is purely what gets persisted. This production change deserves its own test and ideally its own PR (#1).

Worth addressing

4. SkipTest relies on fragile substring matching of file paths (fileName.Contains(\"requeuing\"), \"with_delay\", \"no_broker_created\", ...). Renaming a template silently changes which tests are generated, with no compile-time safety. Consider explicit per-template capability metadata, and at minimum a unit test per branch so a rename can't quietly drop coverage.

5. ADR 0037 number collision. docs/adr/0037-add-messaging-gateway-generated-test.md is the fifth file numbered 0037 (master already has aws-test-resource-cleanup, provide-roslyn-analyzers, reject-message-on-error-handler, universal-scheduler-delay). The earlier collisions aren't yours, but please bump this one to the next free number so the ADR log stays navigable.

Minor / nits

• UTF-8 BOM introduced into MsSqlMessageConsumer.cs (the first using line gains a BOM) and the new AzureServiceBusMessageCreator.cs. Harmless but inconsistent — worth stripping.
• Renamed AzureServiceBusMessageCreator (nice fix on the Messsage typo) is still referenced via a field named _azureServiceBusMesssageCreator — the typo lingers in the field name.
• Default Receive timeout of 300ms in the templates is tight for real brokers; given the history of per-gateway timeout tuning, just a heads-up that the generated single-Receive (no retry loop) tests may be flaky on slower CI.

Verified as legitimate fixes

• Postgres CommandTimeout = timeOut.Value.Seconds -> .TotalSeconds is a real bug fix (a 90s timeout previously became 30s); confirmed both call sites corrected.
• Preserving OriginalMessageIdHeaderName in the bag on requeue across the MsSql/Postgres consumers is sensible and consistently applied.
• The BaseGenerator default-application override pattern and SharedGenerator defaulting are clean and consistent with OutboxGenerator.

Overall: solid, well-documented work that's hard to review only because of its breadth. Main asks: (2) generator unit tests, (3) clarify the MsSql DateTime downgrade, (1) peel the production src/ changes into their own PR(s).

Automated review — suggestions, not blocking gates.

[claude]

Code Review — PR #3996: Messaging Gateway test generation

Thanks for this — extending Paramore.Brighter.Test.Generator to standardize messaging-gateway tests (Reactor + Proactor) is a nice consistency win, and the design closely mirrors the existing OutboxGenerator, which makes the new code easy to follow. The XML docs added across the configuration/generator classes are a genuine improvement. The bulk of the +48k lines is generated output; my review focuses on the hand-written generator code, templates, and the test surface.

🔴 Test coverage — the main gap

The repo already has tests/Paramore.Brighter.Test.Generator.Tests/ with dedicated folders for OutboxGenerator, SharedGenerator, and Parser (e.g. When_multiple_outboxes_missing_prefix_should_use_key_as_prefix, When_outbox_configuration_missing_message_factory_should_use_parent_factory). The new MessagingGatewayGenerator ships with no corresponding tests, even though it contains the most logic-heavy new code in the PR:

• SkipTest(...) — 7 feature-flag → filename-substring rules
• single (MessagingGateway) vs. multiple (MessagingGateways) dispatch
• the protected GenerateAsync override that back-fills MessageBuilder / Namespace / MessageAssertion defaults from the parent config

Given CLAUDE.md's mandatory TDD workflow and the established test pattern for the analogous OutboxGenerator, a MessagingGatewayGenerator/ test folder mirroring those cases (missing-prefix-uses-key, missing-namespace-inherits-parent, each SkipTest flag) would close the gap. SkipTest in particular is exactly the substring-matching logic that silently rots — e.g. the requeuing rule also matches When_requeuing_a_message_too_many_times_should_move_to_dead_letter_queue, so a gateway with HasSupportToRequeue=false skips the DLQ test too. That may be intended, but it is the kind of coupling a test should pin down.

🟡 Design / correctness notes

1. DefaultMessageBuilder is a stateful, non-resetting builder reused across Build() calls. In When_a_message_consumer_reads_multiple_messages... a single _messageBuilder builds 4 messages. The distinguishing fields (Topic/MessageId/Body) are set per-message so the test works, but every built Message shares the same reference-type state — notably the _bag Dictionary and _baggage Baggage instances. All four messages alias one Bag. It is benign for the current assertions, but it is a latent foot-gun for anyone adding a test that mutates the bag. Consider documenting that Build() does not snapshot/reset, or cloning reference-type state inside Build().

2. Publish-confirmation polling reads a field written from another thread without synchronization. In When_confirming_posting_a_message_should_receive_publish_confirmation, messageSent is set in the OnMessagePublished handler and read in a while loop. With Task.Delay/Thread.Sleep between reads it will converge in practice, but messageSent is not volatile/guarded — worth a volatile or a TaskCompletionSource to make intent explicit and avoid a theoretically hoisted read.

3. Single vs. multiple prefix handling differs (mirrors the existing Outbox quirk). For MessagingGateways (dictionary), the prefix becomes $".{prefix}", yielding namespaces like MessagingGateway.RabbitMq.Reactor. For a single MessagingGateway, Prefix is used verbatim with no leading dot, so a non-empty single prefix produces MessagingGatewayRabbitMq.Reactor. This matches OutboxGenerator behavior, so it is consistent — but the difference is non-obvious and the single-config user must include their own . to match. A line in the Prefix XML doc would save confusion.

4. MessageFactory → MessageBuilder is a silent-fallback rename. The config property was renamed (and DefaultMessageFactory → DefaultMessageBuilder + new DefaultMessageAssertion). Because System.Text.Json ignores unknown properties by default, any pre-existing test-configuration.json still using messageFactory will silently fall back to the default rather than erroring. Acceptable for an internal tool, but worth calling out in the ADR/changelog.

5. Dead no-op call retained in the single-Outbox path. OutboxGenerator.GenerateAsync still calls GenerateAsync(configuration, "Outbox", "Outbox", configuration.Outbox), but Templates/Outbox/ contains only Sync/ and Async/ subfolders (no top-level *.liquid), so this call always finds 0 files. The multiple-outbox path correctly dropped its equivalent call. Removing it from the single path too would avoid the misleading "Found 0 liquid files" log.

🟢 Minor / nits

• Generated XML-doc typos (carried into every generated provider): IAmAMessageGatewayReactorProvider says "An synchronous message producer" (→ "A synchronous") and the CreateChannel summary says "asynchronous channel" for a sync method. Cheap to fix once in the template, multiplied across all gateways.
• DefaultMessageAssertion hardcodes Assert.Equal(TimeSpan.Zero, actual.Header.Delayed) — a reasonable default, but it bakes in the assumption that no received message reports a delay. Gateways that surface delay metadata on receive would need a custom MessageAssertion; worth a comment so it is discoverable.
• Whole-file reformatting churn (CSharpier-style — parens/args on their own lines in Program.cs, OutboxGenerator.cs, SharedGenerator.cs). It inflates the diff and mixes pure-formatting changes with functional ones, which makes review harder and slightly rubs against CLAUDE.md's "restrict changes to exactly what was asked" guidance. Not blocking, but a formatting-only commit separated from behavior would be ideal next time.
• Directory.Build.props adding net10.0 and the csproj switch to TargetFrameworks + Nullable enable look correct and consistent with the multi-target intent.

Summary

Solid, well-structured extension that fits the existing generator architecture. The one thing I would treat as a should-fix before merge is unit tests for MessagingGatewayGenerator (especially SkipTest), to match the existing OutboxGenerator test coverage and the project's TDD requirements. The rest are correctness hardening and documentation nits.

🤖 Automated review — verify findings against your own judgment.

[iancooper]

Nice. The solution with the wrapper channel to the problems of delay and retry for Kafka is neatly thought out

[codescene-delta-analysis]

Gates Failed
New code is healthy (1 new file with code health below 9.00)
Enforce advisory code health rules (1 file with Code Duplication)

Our agent can fix these. Install it.

Gates Passed
2 Quality Gates Passed

Reason for failure

New code is healthy	Violations	Code Health Impact
AzureServiceBusMessageCreator.cs	1 rule	8.82	Suppress

Enforce advisory code health rules	Violations	Code Health Impact
AzureServiceBusMessageCreator.cs	1 advisory rule	8.82	Suppress

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

[claude]

Code Review — PR #3996: Messaging Gateway Test Generation

Thanks for this — extending Paramore.Brighter.Test.Generator to standardize messaging-gateway tests is a genuinely valuable direction, and the implementation mirrors the existing OutboxGenerator architecture nicely. The new config classes and templates are well-documented with XML comments and an ADR is included. 👍

I focused the review on the core generator source (tools/Paramore.Brighter.Test.Generator/), the new Liquid templates, and the resulting test-project changes (the PR is ~1,100 files / ~49k insertions, much of it regenerated output and .claude/.agent_instructions docs).

🔴 Stale generated files left behind by the MessageFactory → MessageBuilder rename (cleanup / correctness)

The rename from IAmAMessageFactory/DefaultMessageFactory to IAmAMessageBuilder/DefaultMessageBuilder regenerated the new files but did not remove the old ones (the generator only writes, never deletes). The result:

• MessageFactory.cs (the old IAmAMessageFactory interface) and DefaultMessageFactory.cs are still present in 8 test projects — MSSQL, PostgresSQL, Sqlite, DynamoDB, DynamoDB.V4, MongoDb, MySQL, Gcp.

• In 7 of those 8 projects the old types have zero references outside their own definitions (IAmAMessageFactory is referenced only by DefaultMessageFactory, which itself is referenced nowhere). They are pure dead code now that all generated outbox tests use DefaultMessageBuilder/IAmAMessageBuilder (verified: 104 DefaultMessageBuilder refs vs 0 factory refs in e.g. Sqlite/PostgresSQL outbox tests).

• In MSSQL the migration is incomplete: two hand-written tests still depend on DefaultMessageFactory:

  • Outbox/Text/When_Retrieving_Dispatched_Messages_Async_With_Page_Size_Smaller_Than_Total_Rows_It_Should_Return_Eligible_Messages.cs
  • Outbox/Text/When_Retrieving_Dispatched_Messages_With_Page_Size_Smaller_Than_Total_Rows_It_Should_Return_Eligible_Messages.cs

  These weren't regenerated, so MSSQL now carries both the old factory and the new builder.

Recommendation: delete the orphaned MessageFactory.cs/DefaultMessageFactory.cs from the 7 fully-migrated projects, and migrate the two MSSQL stragglers to the builder so the old files can be removed everywhere. It would also be worth teaching the generator (or a build step) to prune outputs whose templates no longer exist, so this doesn't recur on the next rename.

🟡 Single-gateway Prefix yields a malformed namespace

In MessagingGatewayGenerator.GenerateAsync, the multiple-gateways path normalizes the prefix with a leading dot:

messagingGatewayConfiguration.Prefix = $".{prefix}";

but the single MessagingGateway path uses the raw Prefix value. The templates render namespace {{ Namespace }}.MessagingGateway{{ Prefix }}.Reactor;. So if a user sets Prefix = "Rmq" on the single MessagingGateway config, the namespace becomes ...MessagingGatewayRmq.Reactor (missing separator) rather than ...MessagingGateway.Rmq.Reactor. The default empty prefix is fine, and OutboxGenerator has the same pre-existing pattern, but it is an easy footgun — consider applying the same .{prefix} normalization (or string.Empty) in both paths.

🟡 Implicit ordering coupling between generators

SharedGenerator mutates the root configuration.MessageBuilder/MessageAssertion to their defaults, and OutboxGenerator/MessagingGatewayGenerator then read those as fallbacks. This only works because Program.cs runs Shared first. It is a fragile implicit dependency — if the order ever changes, gateway/outbox configs silently lose their defaults. Consider resolving defaults in one place (e.g., a normalization step on TestConfiguration) rather than as a side effect of the shared generator.

🟡 Hardcoded timing in the delayed-delivery template (flakiness)

When_reading_a_delayed_message... hardcodes SendWithDelay(message, TimeSpan.FromSeconds(5)) and Thread.Sleep(TimeSpan.FromSeconds(5)). Sleeping exactly the delay duration is borderline and can be flaky under load; everything else in the templates is configurable via MessagingGatewayConfiguration (timeouts, inter-receive delay), so it would be consistent to make the delay configurable and sleep delay + buffer.

🟢 Minor notes

• DefaultMessageAssertion hardcodes Assert.Equal(TimeSpan.Zero, actual.Header.Delayed). For the delayed-delivery test this assumes the received message Delayed value is reset by the transport. It holds for SQS and is overridable per-config, but it is a built-in assumption worth a comment.
• Nullable warnings in generated tests. e.g. the dead-letter reactor test declares Message? received; then calls _channel.Requeue(received) and received.Header... without a null check; depending on the test project <Nullable> setting this may emit CS8604/CS8602 warnings. A received! or Assert.NotNull would keep generated code warning-clean.
• SkipTest substring matching is correct — I checked every Contains(...) token (confirming_posting, delayed_message, with_delay, dead_letter_queue, no_broker_created, requeuing, assume_channel, validate_channel) against all 13 template filenames in both Reactor and Proactor folders; all match as intended.
• PR scope. Bundling the generator feature, a mass outbox-test regeneration, and a large set of .claude/.agent_instructions tooling docs into one PR makes it hard to review and is likely how the stale-file issue above slipped through. Splitting these would help reviewers and reduce risk.

Summary

The core design is sound and consistent with the existing generator. The main thing to address before merge is the stale MessageFactory/DefaultMessageFactory cleanup (8 projects) and the two unmigrated MSSQL tests; the prefix-normalization and generator-ordering items are smaller robustness improvements.

Review generated with assistance from Claude Code.