Clean up and improve recoverability message failure acceptance tests

src/ServiceControl.AcceptanceTests/Licensing/When_creating_a_usage_report_on_a_broker_transport.cs

@@ -24,8 +24,6 @@ namespace ServiceControl.AcceptanceTests.Licensing
     using Particular.LicensingComponent.Shared;
     using ServiceControl.Transports.BrokerThroughput;
 
-    // The same journey as When_creating_a_usage_report_on_a_non_broker_transport, on the branch every
-    // production install except MSMQ takes. 
     class When_creating_a_usage_report_on_a_broker_transport : AcceptanceTest
     {
         [Test]
@@ -63,8 +61,6 @@ await Define<Context>()
                 })
                 .Do("List the endpoints reporting throughput", async _ =>
                 {
-                    // Monitoring reports the same endpoint the broker measured, so wait for both
-                    // before reading the list, otherwise the grouping below proves nothing.
                     var summary = await this.TryGet<List<EndpointThroughputSummary>>(
                         "/api/licensing/endpoints",
                         items => items.Any(item => item.MaxDailyThroughput == BrokerThroughput));
@@ -99,9 +95,6 @@ await Define<Context>()
                 Assert.That(reportData.GetProperty("ReportMethod").GetString(), Is.EqualTo("Broker"),
                     "Particular reads the report method to know how the numbers were measured");
 
-                // The broker calls the queue Contoso/Sales and monitoring calls the endpoint the same
-                // thing, but the two only line up once the broker's sanitized name is applied to both.
-                // Without that they are two endpoints, and the customer's report counts them twice.
                 Assert.That(endpoints, Has.Exactly(1).Items,
                     "The broker queue and the monitored endpoint are one endpoint, not two");
 
@@ -124,8 +117,7 @@ await Define<Context>()
             }
         }
 
-        // The collector waits 40 seconds before its first pass, which is longer than this scenario
-        // should take, and the delay is only reachable through the registration.
+        // The collector waits 40 seconds before its first pass, reachable only through the registration.
         static void CollectFromTheBrokerImmediately(IHostApplicationBuilder builder)
         {
             var scheduled = builder.Services.Single(registration =>
@@ -161,7 +153,6 @@ class Context : ScenarioContext, ISequenceContext
             public int Step { get; set; }
         }
 
-        // Reports the same endpoint the broker measured, under the name monitoring knows it by.
         class MonitoringInstance : EndpointConfigurationBuilder
         {
             public MonitoringInstance() =>

src/ServiceControl.AcceptanceTests/Licensing/When_creating_a_usage_report_on_a_non_broker_transport.cs

@@ -18,9 +18,6 @@ namespace ServiceControl.AcceptanceTests.Licensing
     using Particular.LicensingComponent.MonitoringThroughput;
     using Particular.LicensingComponent.Shared;
 
-    // The journey ServicePulse's throughput page walks a user through: see whether a report is
-    // possible, review where the numbers come from, correct what counts as an endpoint, redact the
-    // names that cannot leave the building, and download the report to send to Particular.
     class When_creating_a_usage_report_on_a_non_broker_transport : AcceptanceTest
     {
         [Test]
@@ -135,8 +132,6 @@ class Context : ScenarioContext, ISequenceContext
             public int Step { get; set; }
         }
 
-        // Stands in for the monitoring instance, which reports what it saw to the primary instance's
-        // throughput queue.
         class MonitoringInstance : EndpointConfigurationBuilder
         {
             public MonitoringInstance() =>
@@ -146,8 +141,7 @@ class ReportThroughput : DispatchRawMessages<Context>
             {
                 protected override TransportOperations CreateMessage(Context context)
                 {
-                    // Yesterday: a report only counts complete days, so throughput recorded for today
-                    // is deliberately not enough to generate one.
+                    // Yesterday, because a usage report only counts complete days.
                     var recorded = new RecordEndpointThroughputData
                     {
                         StartDateTime = DateTime.UtcNow.AddDays(-1).AddHours(-1),

src/ServiceControl.AcceptanceTests/Monitoring/When_an_unmonitored_endpoint_is_marked_as_monitored.cs

@@ -60,7 +60,6 @@ await Define<MyContext>()
                                 MonitorHeartbeat = true
                             });
                             state = State.WaitingForHeartbeatFailure;
-                            Console.WriteLine("Patch successful");
                         }
 
                         return false;

src/ServiceControl.AcceptanceTests/Recoverability/Groups/When_a_message_fails_twice_with_different_exceptions.cs

@@ -78,7 +78,6 @@ await Define<MeowContext>()
 
             foreach (var failureId in originalExceptionAndStackTraceFailureGroupIds)
             {
-                Console.WriteLine($"failureId: {failureId}");
                 Assert.That(retriedExceptionAndStackTraceFailureGroupIds, Does.Not.Contain(failureId), $"Failure Group {failureId} is still set on retried message");
             }
         }

src/ServiceControl.AcceptanceTests/Recoverability/Groups/When_messages_have_failed.cs

@@ -3,7 +3,6 @@
     using System;
     using System.Collections.Generic;
     using System.Linq;
-    using System.Text.Json;
     using System.Threading.Tasks;
     using AcceptanceTesting;
     using AcceptanceTesting.EndpointTemplates;
@@ -74,7 +73,6 @@ public async Task Should_be_grouped()
                 Assert.That(messageTypeGroups.Count, Is.EqualTo(2), "There should be 2 Message Type Groups");
             }
 
-            defaultGroups.ForEach(g => Console.WriteLine(JsonSerializer.Serialize(g)));
 
             Assert.That(exceptionTypeAndStackTraceGroups.Select(g => g.Id).Except(defaultGroups.Select(g => g.Id)), Is.Empty, "/api/recoverability/groups did not retrieve Exception Type and Stack Trace Group");
 

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/ErrorImportPerformanceTests.cs

@@ -19,22 +19,13 @@ class ErrorImportPerformanceTests : AcceptanceTest
         public async Task Should_import_all_messages(CancellationToken cancellationToken = default)
         {
             await Define<MyContext>()
-                .WithEndpoint<Receiver>(b => b.When(bus => Task.WhenAll(Enumerable.Repeat(0, 100).Select(i => bus.SendLocal(new MyMessage())))).DoNotFailOnErrorMessages())
+                .WithEndpoint<Receiver>(b => b.When(bus => Task.WhenAll(Enumerable.Repeat(0, ExpectedMessages).Select(i => bus.SendLocal(new MyMessage())))).DoNotFailOnErrorMessages())
                 .Done(async c =>
                 {
                     var result = await this.TryGetMany<MessagesView>("/api/messages?per_page=150");
-                    if (!result)
-                    {
-                        return false;
-                    }
-
-                    List<MessagesView> messages = result;
-                    if (messages.Count < 100)
-                    {
-                        Console.Out.WriteLine("Messages found: " + messages.Count);
-                    }
-
-                    return messages.Count >= 100;
+                    c.MessagesImported = result ? ((List<MessagesView>)result).Count : 0;
+
+                    return c.MessagesImported >= ExpectedMessages;
                 })
                 .Run(cancellationToken);
         }
@@ -53,6 +44,11 @@ public class MyMessageHandler : IHandleMessages<MyMessage>
 
         public class MyMessage : ICommand;
 
-        public class MyContext : ScenarioContext;
+        const int ExpectedMessages = 100;
+
+        public class MyContext : ScenarioContext
+        {
+            public int MessagesImported { get; set; }
+        }
     }
 }

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_failed_message_is_pending_retry.cs

@@ -23,31 +23,20 @@ public async Task Should_status_retryissued_after_retry_is_sent()
             FailedMessage failedMessage = null;
 
             await Define<Context>()
-                .WithEndpoint<FailingEndpoint>(b => b.When(async ctx =>
-                {
-                    if (ctx.UniqueMessageId == null)
-                    {
-                        return false;
-                    }
-
-                    var result = await this.TryGet<FailedMessage>($"/api/errors/{ctx.UniqueMessageId}");
-                    failedMessage = result;
-                    return result;
-                }, async (bus, ctx) =>
+                .WithEndpoint<FailingEndpoint>(b => b.DoNotFailOnErrorMessages())
+                .Do("Wait for the message to fail", async ctx =>
+                    ctx.UniqueMessageId != null && await this.TryGet<FailedMessage>($"/api/errors/{ctx.UniqueMessageId}"))
+                .Do("Issue a retry", async ctx =>
                 {
                     ctx.AboutToSendRetry = true;
                     await this.Post<object>($"/api/errors/{ctx.UniqueMessageId}/retry");
-                }).DoNotFailOnErrorMessages())
-                .Done(async ctx =>
+                })
+                .Do("Wait for the retry to be handled", ctx => Task.FromResult(ctx.Retried))
+                .Do("Read the failed message back", async ctx =>
                 {
-                    if (ctx.Retried)
-                    {
-                        failedMessage = await this.TryGet<FailedMessage>($"/api/errors/{ctx.UniqueMessageId}");
-                        return true;
-                    }
-
-                    return false;
+                    failedMessage = await this.TryGet<FailedMessage>($"/api/errors/{ctx.UniqueMessageId}");
                 })
+                .Done()
                 .Run();
 
             Assert.That(failedMessage.Status, Is.EqualTo(FailedMessageStatus.RetryIssued), "Status was not set to RetryIssued");
@@ -81,7 +70,6 @@ public class MyMessageHandler(Context scenarioContext, IReadOnlySettings setting
             {
                 public Task Handle(MyMessage message, IMessageHandlerContext context)
                 {
-                    Console.WriteLine("Message Handled");
                     if (scenarioContext.AboutToSendRetry)
                     {
                         scenarioContext.Retried = true;
@@ -97,11 +85,12 @@ public Task Handle(MyMessage message, IMessageHandlerContext context)
             }
         }
 
-        public class Context : ScenarioContext
+        public class Context : ScenarioContext, ISequenceContext
         {
             public string UniqueMessageId { get; set; }
             public bool Retried { get; set; }
             public bool AboutToSendRetry { get; set; }
+            public int Step { get; set; }
         }
 
         public class MyMessage : ICommand;

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_invalid_id_is_sent_to_retry.cs

@@ -1,6 +1,7 @@
 namespace ServiceControl.AcceptanceTests.Recoverability.MessageFailures
 {
     using System;
+    using System.Net;
     using System.Threading;
     using System.Threading.Tasks;
     using AcceptanceTesting;
@@ -17,30 +18,27 @@ class When_a_invalid_id_is_sent_to_retry : AcceptanceTest
         [CancelAfter(180_000)]
         public async Task SubsequentBatchesShouldBeProcessed(CancellationToken cancellationToken = default)
         {
-            var context = await Define<MyContext>()
+            HttpStatusCode retryOfUnknownId = default;
+
+            await Define<MyContext>()
                 .WithEndpoint<FailureEndpoint>(cfg => cfg
                     .When(async bus =>
                     {
-                        while (true)
-                        {
-                            try
-                            {
-                                await this.Post<object>("/api/errors/1785201b-5ccd-4705-b14e-f9dd7ef1386e/retry");
-                                break;
-                            }
-                            catch (InvalidOperationException)
-                            {
-                                // api not up yet
-                            }
-                        }
+                        using var response = await HttpClient.PostAsync($"/api/errors/{UnknownFailedMessageId}/retry", null, cancellationToken);
+
+                        retryOfUnknownId = response.StatusCode;
 
                         await bus.SendLocal(new MessageThatWillFail());
-                    }).DoNotFailOnErrorMessages()
-                    .When(async ctx => ctx.IssueRetry && await this.TryGet<object>("/api/errors/" + ctx.UniqueMessageId), (bus, ctx) => this.Post<object>($"/api/errors/{ctx.UniqueMessageId}/retry")).DoNotFailOnErrorMessages())
-                .Done(ctx => ctx.Done)
+                    }).DoNotFailOnErrorMessages())
+                .Do("Wait for the message to fail", async ctx =>
+                    ctx.IssueRetry && await this.TryGet<object>($"/api/errors/{ctx.UniqueMessageId}"))
+                .Do("Retry the failed message", async ctx =>
+                    await this.Post<object>($"/api/errors/{ctx.UniqueMessageId}/retry"))
+                .Do("Wait for the retry to be handled", ctx => Task.FromResult(ctx.Done))
+                .Done()
                 .Run(cancellationToken);
 
-            Assert.That(context.Done, Is.True);
+            Assert.That(retryOfUnknownId, Is.EqualTo(HttpStatusCode.Accepted));
         }
 
         public class FailureEndpoint : EndpointConfigurationBuilder
@@ -70,15 +68,18 @@ public Task Handle(MessageThatWillFail message, IMessageHandlerContext context)
             }
         }
 
-        public class MyContext : ScenarioContext
+        public class MyContext : ScenarioContext, ISequenceContext
         {
+            public int Step { get; set; }
             public bool Done { get; set; }
             public bool ExceptionThrown { get; set; }
             public bool IssueRetry { get; set; }
             public string UniqueMessageId { get; set; }
         }
 
 
+        const string UnknownFailedMessageId = "1785201b-5ccd-4705-b14e-f9dd7ef1386e";
+
         public class MessageThatWillFail : ICommand;
     }
 }

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_message_has_failed.cs

@@ -232,14 +232,12 @@ public Receiver() =>
             [Handler]
             public class MyMessageHandler(
                 MyContext testContext,
-                IReadOnlySettings settings,
-                ReceiveAddresses receiveAddresses)
+                IReadOnlySettings settings)
                 : IHandleMessages<MyMessage>
             {
                 public Task Handle(MyMessage message, IMessageHandlerContext context)
                 {
                     testContext.EndpointNameOfReceivingEndpoint = settings.EndpointName();
-                    testContext.LocalAddress = receiveAddresses.MainReceiveAddress;
                     testContext.MessageId = context.MessageId.Replace(@"\", "-");
                     throw new Exception("Simulated exception");
                 }
@@ -258,14 +256,12 @@ public ReceiverWithCustomSerializer() =>
             [Handler]
             public class MyMessageHandler(
                 MyContext testContext,
-                IReadOnlySettings settings,
-                ReceiveAddresses receiveAddresses)
+                IReadOnlySettings settings)
                 : IHandleMessages<MyMessage>
             {
                 public Task Handle(MyMessage message, IMessageHandlerContext context)
                 {
                     testContext.EndpointNameOfReceivingEndpoint = settings.EndpointName();
-                    testContext.LocalAddress = receiveAddresses.MainReceiveAddress;
                     testContext.MessageId = context.MessageId.Replace(@"\", "-");
                     throw new Exception("Simulated exception");
                 }
@@ -328,7 +324,6 @@ public class MyContext : ScenarioContext
             public string MessageId { get; set; }
             public string EndpointNameOfReceivingEndpoint { get; set; }
             public string UniqueMessageId => DeterministicGuid.MakeId(MessageId, EndpointNameOfReceivingEndpoint).ToString();
-            public string LocalAddress { get; set; }
         }
 
         public class QueueSearchContext : ScenarioContext

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_queue_and_timeframe.cs

@@ -76,7 +76,6 @@ public class MyMessageHandler(
             {
                 public Task Handle(MyMessage message, IMessageHandlerContext context)
                 {
-                    Console.WriteLine("Message Handled");
                     if (scenarioContext.Step == 0)
                     {
                         scenarioContext.FromAddress = receiveAddresses.MainReceiveAddress;
@@ -85,7 +84,6 @@ public Task Handle(MyMessage message, IMessageHandlerContext context)
                     }
 
                     scenarioContext.RetryCount++;
-                    scenarioContext.Retried = true;
 
                     return Task.CompletedTask;
                 }
@@ -95,7 +93,6 @@ public Task Handle(MyMessage message, IMessageHandlerContext context)
         public class Context : ScenarioContext, ISequenceContext
         {
             public string UniqueMessageId { get; set; }
-            public bool Retried { get; set; }
             public int RetryCount { get; set; }
             public string FromAddress { get; set; }
             public int Step { get; set; }

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_selection.cs

@@ -62,22 +62,18 @@ public FailingEndpoint() =>
             [Handler]
             public class MyMessageHandler(
                 Context scenarioContext,
-                IReadOnlySettings settings,
-                ReceiveAddresses receiveAddresses)
+                IReadOnlySettings settings)
                 : IHandleMessages<MyMessage>
             {
                 public Task Handle(MyMessage message, IMessageHandlerContext context)
                 {
-                    Console.WriteLine("Message Handled");
                     if (scenarioContext.Step == 0)
                     {
-                        scenarioContext.FromAddress = receiveAddresses.MainReceiveAddress;
                         scenarioContext.UniqueMessageId = DeterministicGuid.MakeId(context.MessageId, settings.EndpointName()).ToString();
                         throw new Exception("Simulated Exception");
                     }
 
                     scenarioContext.RetryCount++;
-                    scenarioContext.Retried = true;
 
                     return Task.CompletedTask;
                 }
@@ -87,9 +83,7 @@ public Task Handle(MyMessage message, IMessageHandlerContext context)
         public class Context : ScenarioContext, ISequenceContext
         {
             public string UniqueMessageId { get; set; }
-            public bool Retried { get; set; }
             public int RetryCount { get; set; }
-            public string FromAddress { get; set; }
             public int Step { get; set; }
         }