diff --git a/docs/acceptance-test-review-plan.md b/docs/acceptance-test-review-plan.md index fae3d0dcb0..009d664691 100644 --- a/docs/acceptance-test-review-plan.md +++ b/docs/acceptance-test-review-plan.md @@ -10,9 +10,9 @@ That is the shape of the problem: these failures are silent. A test that asserts ## Where this has got to -Phases 1 and 2 are done. Phase 3 has swept the first of its four areas, `Recoverability/MessageFailures`. Phase 4 has closed the licensing block, 8 routes of the 40, on both of the branches those routes take. +Phases 1, 2 and 3 are done. Phase 4 has closed the licensing block, 8 routes of the 40, on both of the branches those routes take. -What is left: three areas to sweep, of which the two `ExternalIntegration` ones are parked until the EF work lands, and 32 routes to cover. +What is left: 32 routes to cover, starting with the three nav-gating ones. ## Scope @@ -42,13 +42,13 @@ The double is registered as its concrete type, so the collection the production Seven files contain no `Assert` at all. Five use the `Do("step", …)` sequence helper, which logs `Advancing from X to Y` on each transition, so a regression there is diagnosable from the console output. That is a deliberate and acceptable pattern. The other two gated on a single `Done` predicate and reported a regression as a bare timeout with nothing to read. -**Detect:** files with a `[Test]` and zero `Assert.` occurrences, minus those using the `Sequence` helper. Seven have no assertion, five of them legitimately. `ErrorImportPerformanceTests` is fixed: it records the count on the context, which the runner prints when a scenario does not finish. That leaves `When_single_message_fails_in_batch`, in the area phase 3 sweeps next. +**Detect:** files with a `[Test]` and zero `Assert.` occurrences, minus those using the `Sequence` helper. Outside `ExternalIntegration` this is now clear: eight files have no assertion, seven of them driven by steps, and `ErrorImportPerformanceTests` records its count on the context, which the runner prints when a scenario does not finish. ### 3. Assertion restates the condition the scenario already waited on (unfalsifiable) `When_a_invalid_id_is_sent_to_retry` ended with `Assert.That(context.Done, Is.True)` after `.Done(ctx => ctx.Done)`. That assertion could not fail: if the flag were false the scenario would have timed out first. The real subject of the test, that posting a retry for a non-existent id does not break subsequent batches, was never asserted, and the response of the invalid POST was never inspected. -**Detect:** a mechanical grep finds candidates of the shape `Assert.That(context.Flag, Is.True)`, currently 9. The confirmed one is fixed and now asserts the status the unknown id answers with. The rest need reading individually, because a flag set by a message handler and gated on something else is legitimate: `When_failed_message_searched_by_body_content` was read and is sound, since its `Done` returns true whether or not the flag is set. Four of the nine are in `ExternalIntegration`, which is parked. +**Detect:** a mechanical grep finds candidates of the shape `Assert.That(context.Flag, Is.True)`. Nine were confirmed and fixed across the suite. The two that remain were read and are sound, because a flag set by a message handler while the scenario waits for something else can fail: `When_failed_message_searched_by_body_content`, whose `Done` returns true either way, and `When_single_message_fails_in_batch`, whose new assertion is falsifiable and was proven so. The grep is a starting point, not a verdict. ### 4. Assertions coupled to one persister's internals (portability) @@ -220,9 +220,24 @@ Read for the five patterns above, area by area, so each PR stays reviewable and Thirteen `Console.WriteLine` calls went, across eleven files. Six were a bare "Message Handled" in a handler, which carries no identity and fires on every delivery, so in tests turning on how many times a message was handled it cannot tell the first attempt from the retry. Worse, they sat next to the counter that does answer that, and the runner already prints the context on failure. The rest either narrated a step that throws with detail when it fails, or dumped state next to an assertion whose message says the same thing. They read as debugging left in place rather than diagnostics anyone chose. Not every test wants this. A test that sends a message, waits for one thing and asserts reads worse as a sequence, which is most of `When_a_message_has_failed`. A step also has no `bus`, so anything that sends has to stay an endpoint `When`. -- [ ] `Recoverability/*` root, `Groups`, `MessageRedirects`: the retry, group and redirect flows. -- [ ] `Monitoring/*` and `EventLogs`: heartbeats, custom checks, endpoint monitoring. -- [ ] `Recoverability/ExternalIntegration` and `Monitoring/ExternalIntegration`: hold until the EF external-integration work lands, then review against both persisters at once. + + Converting to steps exposed a second tautology underneath the first. Four of these tests polled until the message was `Resolved` and then asserted it was `Resolved`, which cannot fail: if the status never changes the scenario times out and the assertion never runs. Three were dropped, since the step named "Wait for it to be resolved" reports the same failure with the same precision. The event log test keeps its assertions because they check the description and the related message id, which the poll does not. +- [x] `Recoverability/*` root, `Groups`, `MessageRedirects`, and `Monitoring/*` and `EventLogs`: swept together, since the mechanical passes cover both in one go and the findings were thin. + + `When_single_message_fails_in_batch` was the last test in the suite with no assertion and no sequence to read on failure, and it is one of the two that started this review. Nothing in it checked that its double ran, so the original bug would still pass today. It now asserts the enricher threw, which is not gated on by the scenario: registering the double as its concrete type again fails the test in six seconds with "The enricher never threw, so nothing in the batch failed and the test proved nothing". The imported count also goes on the context so a timeout says how far ingestion got. + + Two more unfalsifiable assertions, both fixed by making the sequence explicit and dropping the restatement: `When_a_message_without_a_correlationid_header_is_retried` asserted the flag its own `Done` waited for, and `MessageRedirects/When_a_message_is_retried` asserted `Received` after `Done(ctx => ctx.Received)`. + + One dead context property, `EmailDropPath`, written by the scenario while the test read the local it was copied from. + + Nothing for the registration or persister-coupling patterns. The four registrations here were settled in phase 2, and every EF exclusion in these areas is in `ExternalIntegration`. +- [x] `Recoverability/ExternalIntegration` and `Monitoring/ExternalIntegration`: reviewed and brought into shape, still excluded from the EF suites until the EF external-integration work lands. + + Six unfalsifiable assertions, the densest pocket in the suite. Every one restated the flag its own `Done` had waited for. Two files already carried a real assertion underneath, so the restatement simply went: `When_a_custom_check_fails` checks the type name external subscribers bind to, and `When_encountered_an_error` checks that the faulty publisher actually ran, which is the point of registering one. + + The other three had nothing else, so removing the restatement would have left them asserting nothing at all. They now assert the same contract their sibling does, the `EnclosedMessageTypes` header of the published event, which is what an external subscriber binds to and what a rename would silently break. Renaming the expected type fails them with the old and new names side by side. + + These are single-wait tests, so none became a sequence. Nothing found for the registration, timeout or rot patterns. ### Phase 4: work through the route list diff --git a/src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_email_notifications_are_enabled.cs b/src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_email_notifications_are_enabled.cs index d03b1938ba..5cbba441ff 100644 --- a/src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_email_notifications_are_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_email_notifications_are_enabled.cs @@ -36,7 +36,6 @@ public async Task Should_send_custom_check_status_change_emails() await Define(c => { - c.EmailDropPath = emailDropPath; }) .WithEndpoint() .Done(c => @@ -104,7 +103,6 @@ public async Task StartAsync(CancellationToken cancellationToken = default) public class MyContext : ScenarioContext { - public string EmailDropPath { get; set; } } public class EndpointWithFailingCustomCheck : EndpointConfigurationBuilder diff --git a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_fails.cs b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_fails.cs index df22f1f83a..c2d4161a55 100644 --- a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_fails.cs +++ b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_fails.cs @@ -57,8 +57,6 @@ await domainEvents.Raise(new Contracts.CustomChecks.CustomCheckFailed .Done(c => c.CustomCheckFailedReceived) .Run(); - Assert.That(context.CustomCheckFailedReceived, Is.True); - var enclosedType = context.IntegrationEventHeaders[Headers.EnclosedMessageTypes]; Assert.That(enclosedType, Is.EqualTo("ServiceControl.Contracts.CustomCheckFailed, ServiceControl.Contracts")); } diff --git a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_succeeds.cs b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_succeeds.cs index 93634225b7..987acae9b2 100644 --- a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_succeeds.cs +++ b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_a_custom_check_succeeds.cs @@ -1,6 +1,7 @@ namespace ServiceControl.AcceptanceTests.Monitoring.ExternalIntegration { using System; + using System.Collections.Generic; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.EndpointTemplates; @@ -55,7 +56,9 @@ await domainEvents.Raise(new Contracts.CustomChecks.CustomCheckSucceeded .Done(c => c.CustomCheckSucceededReceived) .Run(); - Assert.That(context.CustomCheckSucceededReceived, Is.True); + Assert.That(context.IntegrationEventHeaders[Headers.EnclosedMessageTypes], + Is.EqualTo("ServiceControl.Contracts.CustomCheckSucceeded, ServiceControl.Contracts"), + "External subscribers bind to this type name"); } public class ExternalProcessor : EndpointConfigurationBuilder @@ -76,6 +79,7 @@ public class CustomCheckSucceededHandler(MyContext testContext) : IHandleMessage public Task Handle(CustomCheckSucceeded message, IMessageHandlerContext context) { testContext.CustomCheckSucceededReceived = true; + testContext.IntegrationEventHeaders = context.MessageHeaders; return Task.CompletedTask; } } @@ -84,6 +88,7 @@ public Task Handle(CustomCheckSucceeded message, IMessageHandlerContext context) public class MyContext : ScenarioContext { public bool CustomCheckSucceededReceived { get; set; } + public IReadOnlyDictionary IntegrationEventHeaders { get; set; } } } } \ No newline at end of file diff --git a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_is_restored.cs b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_is_restored.cs index 84d35b7092..573a8079a3 100644 --- a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_is_restored.cs +++ b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_is_restored.cs @@ -1,6 +1,7 @@ namespace ServiceControl.AcceptanceTests.Monitoring.ExternalIntegration { using System; + using System.Collections.Generic; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.EndpointTemplates; @@ -55,7 +56,9 @@ public async Task Should_publish_notification() .Done(c => c.NotificationDelivered) .Run(); - Assert.That(context.NotificationDelivered, Is.True); + Assert.That(context.IntegrationEventHeaders[Headers.EnclosedMessageTypes], + Is.EqualTo("ServiceControl.Contracts.HeartbeatRestored, ServiceControl.Contracts"), + "External subscribers bind to this type name"); } public class ExternalProcessor : EndpointConfigurationBuilder @@ -73,6 +76,7 @@ public class FailureHandler(MyContext testContext) : IHandleMessages IntegrationEventHeaders { get; set; } } } } \ No newline at end of file diff --git a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_loss_is_detected.cs b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_loss_is_detected.cs index 1238822be9..6ac8529663 100644 --- a/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_loss_is_detected.cs +++ b/src/ServiceControl.AcceptanceTests/Monitoring/ExternalIntegration/When_heartbeat_loss_is_detected.cs @@ -1,6 +1,7 @@ namespace ServiceControl.AcceptanceTests.Monitoring.ExternalIntegration { using System; + using System.Collections.Generic; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.EndpointTemplates; @@ -57,7 +58,9 @@ public async Task Should_publish_notification() .Done(c => c.NotificationDelivered) .Run(); - Assert.That(context.NotificationDelivered, Is.True); + Assert.That(context.IntegrationEventHeaders[Headers.EnclosedMessageTypes], + Is.EqualTo("ServiceControl.Contracts.HeartbeatStopped, ServiceControl.Contracts"), + "External subscribers bind to this type name"); } public class ExternalProcessor : EndpointConfigurationBuilder @@ -75,6 +78,7 @@ public class FailureHandler(MyContext testContext) : IHandleMessages IntegrationEventHeaders { get; set; } } } } \ No newline at end of file diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/ExternalIntegration/When_encountered_an_error.cs b/src/ServiceControl.AcceptanceTests/Recoverability/ExternalIntegration/When_encountered_an_error.cs index eb7871618b..89d39b87c1 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/ExternalIntegration/When_encountered_an_error.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/ExternalIntegration/When_encountered_an_error.cs @@ -67,11 +67,7 @@ public async Task Should_restart_dispatch_thread() .Done(c => c.NotificationDelivered) .Run(); - using (Assert.EnterMultipleScope()) - { - Assert.That(context.NotificationDelivered, Is.True); - Assert.That(context.Failed, Is.True); - } + Assert.That(context.Failed, Is.True, "The faulty publisher never ran, so the notification was not delivered in spite of one"); } class FaultyPublisher(MyContext context) : IEventPublisher diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_for_a_failed_message_is_successful.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_for_a_failed_message_is_successful.cs index 4565b14cd5..8bba94714e 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_for_a_failed_message_is_successful.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_for_a_failed_message_is_successful.cs @@ -50,34 +50,26 @@ await Define() [CancelAfter(120_000)] public async Task Should_show_up_as_resolved_when_doing_a_multi_retry(CancellationToken cancellationToken = default) { - FailedMessage failure = null; - await Define() .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) - .Do("Wait for the message to fail", async ctx => (failure = await GetFailedMessage(ctx)) != null) + .Do("Wait for the message to fail", async ctx => await GetFailedMessage(ctx)) .Do("Retry the message by id", ctx => IssueRetry(ctx, () => this.Post("/api/errors/retry", new List { ctx.UniqueMessageId }))) - .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx, result => failure = result)) + .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx)) .Done() .Run(cancellationToken); - - Assert.That(failure.Status, Is.EqualTo(FailedMessageStatus.Resolved)); } [Test] [CancelAfter(120_000)] public async Task Should_show_up_as_resolved_when_doing_a_retry_all(CancellationToken cancellationToken = default) { - FailedMessage failure = null; - await Define() .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) - .Do("Wait for the message to fail", async ctx => (failure = await GetFailedMessage(ctx)) != null) + .Do("Wait for the message to fail", async ctx => await GetFailedMessage(ctx)) .Do("Retry everything", ctx => IssueRetry(ctx, () => this.Post("/api/errors/retry/all"))) - .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx, result => failure = result)) + .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx)) .Done() .Run(cancellationToken); - - Assert.That(failure.Status, Is.EqualTo(FailedMessageStatus.Resolved)); } [Test] @@ -90,7 +82,7 @@ await Define() .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) .Do("Wait for the message to fail", async ctx => (failure = await GetFailedMessage(ctx)) != null) .Do("Retry the group it belongs to", ctx => IssueRetry(ctx, () => this.Post($"/api/recoverability/groups/{failure.FailureGroups.First().Id}/errors/retry"))) - .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx, result => failure = result)) + .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx)) .Done() .Run(cancellationToken); } @@ -99,17 +91,13 @@ await Define() [CancelAfter(120_000)] public async Task Should_show_up_as_resolved_when_doing_a_retry_all_for_the_given_endpoint(CancellationToken cancellationToken = default) { - FailedMessage failure = null; - await Define() .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) - .Do("Wait for the message to fail", async ctx => (failure = await GetFailedMessage(ctx)) != null) + .Do("Wait for the message to fail", async ctx => await GetFailedMessage(ctx)) .Do("Retry everything for the endpoint", ctx => IssueRetry(ctx, () => this.Post($"/api/errors/{ctx.EndpointNameOfReceivingEndpoint}/retry/all"))) - .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx, result => failure = result)) + .Do("Wait for it to be resolved", async ctx => await IsResolved(ctx)) .Done() .Run(cancellationToken); - - Assert.That(failure.Status, Is.EqualTo(FailedMessageStatus.Resolved)); } Task> GetFailedMessage(MyContext c) @@ -122,7 +110,7 @@ Task> GetFailedMessage(MyContext c) return this.TryGet("/api/errors/" + c.UniqueMessageId); } - async Task IsResolved(MyContext c, Action capture) + async Task IsResolved(MyContext c, Action capture = null) { var result = await GetFailedMessage(c); @@ -131,7 +119,7 @@ async Task IsResolved(MyContext c, Action capture) return false; } - capture(result); + capture?.Invoke(result); return result.Item.Status == FailedMessageStatus.Resolved; } diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageRedirects/When_a_message_is_retried.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageRedirects/When_a_message_is_retried.cs index 5235d39c36..88612daf6c 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageRedirects/When_a_message_is_retried.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageRedirects/When_a_message_is_retried.cs @@ -20,36 +20,30 @@ class When_a_message_is_retried_with_a_redirect : AcceptanceTest [CancelAfter(120_000)] public async Task It_should_be_sent_to_the_correct_endpoint(CancellationToken cancellationToken = default) { - var context = await Define() + await Define() .WithEndpoint(b => b.When(bus => bus.SendLocal(new MessageToRetry())) - .When(async ctx => - { - if (ctx.UniqueMessageId == null) - { - return false; - } - - return await this.TryGet($"/api/errors/{ctx.UniqueMessageId}"); - }, async (bus, ctx) => - { - await this.Post("/api/redirects", new RedirectRequest - { - fromphysicaladdress = ctx.FromAddress, - tophysicaladdress = ctx.ToAddress - }, status => status != HttpStatusCode.Created); - - await this.Post($"/api/errors/{ctx.UniqueMessageId}/retry"); - }).DoNotFailOnErrorMessages()) + .DoNotFailOnErrorMessages()) .WithEndpoint(c => c.When((session, ctx) => { ctx.ToAddress = Conventions.EndpointNamingConvention(typeof(ToNewEndpoint)); return Task.CompletedTask; })) - .Done(ctx => ctx.Received) - .Run(cancellationToken); + .Do("Wait for the message to fail", async ctx => + ctx.UniqueMessageId != null && await this.TryGet($"/api/errors/{ctx.UniqueMessageId}")) + .Do("Redirect the queue and retry", async ctx => + { + await this.Post("/api/redirects", new RedirectRequest + { + fromphysicaladdress = ctx.FromAddress, + tophysicaladdress = ctx.ToAddress + }, status => status != HttpStatusCode.Created); - Assert.That(context.Received, Is.True); + await this.Post($"/api/errors/{ctx.UniqueMessageId}/retry"); + }) + .Do("Wait for the new endpoint to receive it", ctx => Task.FromResult(ctx.Received)) + .Done() + .Run(cancellationToken); } public class FromEndpoint : EndpointConfigurationBuilder @@ -87,8 +81,9 @@ public Task Handle(MessageToRetry message, IMessageHandlerContext context) } } - public class Context : ScenarioContext + public class Context : ScenarioContext, ISequenceContext { + public int Step { get; set; } public string UniqueMessageId { get; set; } public string FromAddress { get; set; } public string ToAddress { get; set; } diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/When_a_message_without_a_correlationid_header_is_retried.cs b/src/ServiceControl.AcceptanceTests/Recoverability/When_a_message_without_a_correlationid_header_is_retried.cs index ded8d350fb..e8c927cb42 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/When_a_message_without_a_correlationid_header_is_retried.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/When_a_message_without_a_correlationid_header_is_retried.cs @@ -18,39 +18,26 @@ class When_a_message_without_a_correlationid_header_is_retried : AcceptanceTest [Test] public async Task The_successful_retry_should_succeed() { - var context = await Define() + await Define() .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())) .DoNotFailOnErrorMessages()) - .Done(async ctx => + .Do("Wait for the message to fail", async ctx => + !string.IsNullOrWhiteSpace(ctx.UniqueMessageId) && await this.TryGet($"/api/errors/{ctx.UniqueMessageId}")) + .Do("Retry the message", async ctx => { - if (string.IsNullOrWhiteSpace(ctx.UniqueMessageId)) - { - return false; - } - - if (!ctx.RetryIssued) - { - if (!await this.TryGet($"/api/errors/{ctx.UniqueMessageId}")) - { - return false; - } - - ctx.RetryIssued = true; - await this.Post($"/api/errors/{ctx.UniqueMessageId}/retry"); - return false; - } - - return ctx.RetryHandled; + ctx.RetryIssued = true; + await this.Post($"/api/errors/{ctx.UniqueMessageId}/retry"); }) + .Do("Wait for the retry to be handled", ctx => Task.FromResult(ctx.RetryHandled)) + .Done() .Run(); - - Assert.That(context.RetryHandled, Is.True, "Retry not handled correctly"); } internal class MyMessage : IMessage; - internal class MyContext : ScenarioContext + internal class MyContext : ScenarioContext, ISequenceContext { + public int Step { get; set; } public string UniqueMessageId { get; set; } public bool RetryIssued { get; set; } public bool RetryHandled { get; set; } diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/When_single_message_fails_in_batch.cs b/src/ServiceControl.AcceptanceTests/Recoverability/When_single_message_fails_in_batch.cs index 8ed5d32d0d..b8faaf7da3 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/When_single_message_fails_in_batch.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/When_single_message_fails_in_batch.cs @@ -30,7 +30,7 @@ public async Task Should_import_all_messages() SetSettings = settings => settings.MaximumConcurrencyLevel = maximumConcurrencyLevel; - await Define(ctx => + var context = await Define(ctx => { ctx.MessageId = Guid.NewGuid().ToString(); }) @@ -39,14 +39,19 @@ await Define(ctx => { var result = await this.TryGetMany("/api/messages?include_system_messages=false&sort=id"); List messages = result; - if (result) + if (!result) { - return messages.Count == 9 && messages.Select(m => m.MessageId).OrderBy(t => t).SequenceEqual(c.MessageIds.OrderBy(t => t)); + return false; } - return false; + c.MessagesImported = messages.Count; + + return messages.Count == BatchSize && messages.Select(m => m.MessageId).OrderBy(t => t).SequenceEqual(c.MessageIds.OrderBy(t => t)); }) .Run(); + + Assert.That(context.FailureSimulated, Is.True, + "The enricher never threw, so nothing in the batch failed and the test proved nothing"); } class FailOnceEnricher(MyContext testContext) : IEnrichImportedErrorMessages @@ -55,11 +60,9 @@ public void Enrich(ErrorEnricherContext context) { if (context.Headers[Headers.MessageId] == testContext.MessageId && Interlocked.Increment(ref attempt) == 1) { - TestContext.Out.WriteLine("Simulating message processing failure"); + testContext.FailureSimulated = true; throw new InvalidOperationException("ID", null); } - - TestContext.Out.WriteLine("Message processed correctly"); } int attempt; @@ -74,8 +77,8 @@ class SendMessage : DispatchRawMessages protected override TransportOperations CreateMessage(MyContext context) { // put the message that will fail somewhere in the middle of the first batch - var operations = new TransportOperation[9]; - for (var i = 0; i < 9; i++) + var operations = new TransportOperation[BatchSize]; + for (var i = 0; i < BatchSize; i++) { var headers = new Dictionary { @@ -96,10 +99,14 @@ protected override TransportOperations CreateMessage(MyContext context) } } + const int BatchSize = 9; + public class MyContext : ScenarioContext { public string MessageId { get; set; } public List MessageIds { get; } = []; + public bool FailureSimulated { get; set; } + public int MessagesImported { get; set; } } } } \ No newline at end of file