Acceptance tests small todos (#4001)

* Remove TODO since we can't dynamically change the database settings yet without resolving the database configuration from DI

* Moved some things to test specific overrides

* EndpointConfigurationExtensions for testing

* Remove compile include

* Apply suggestions from code review

Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com>

* Styling

* Align controller loading for ATTs

---------

Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com>

src/ServiceControl.AcceptanceTests/TestSupport/EndpointConfigurationExtensions.cs

@@ -1,12 +1,37 @@
 namespace ServiceControl.AcceptanceTests.TestSupport
 {
+    using System.IO;
+    using AcceptanceTesting;
+    using Microsoft.Extensions.DependencyInjection;
     using NServiceBus;
+    using NServiceBus.AcceptanceTesting;
+    using NServiceBus.Configuration.AdvancedExtensibility;
 
     public static class EndpointConfigurationExtensions
     {
         public static void ReportSuccessfulRetriesToServiceControl(this EndpointConfiguration configuration)
         {
             configuration.Pipeline.Register(typeof(ReportSuccessfulRetryToServiceControl), "Simulate that the audit instance detects and reports successfull retries");
         }
+
+        public static void CustomizeServiceControlEndpointTesting(this EndpointConfiguration configuration, ScenarioContext context)
+        {
+            configuration.GetSettings().Set("SC.ScenarioContext", context);
+            configuration.GetSettings().Set(context);
+
+            configuration.RegisterComponents(r =>
+            {
+                r.AddSingleton(context.GetType(), context);
+                r.AddSingleton(typeof(ScenarioContext), context);
+            });
+
+            configuration.Pipeline.Register<TraceIncomingBehavior.Registration>();
+            configuration.Pipeline.Register<TraceOutgoingBehavior.Registration>();
+            configuration.Pipeline.Register(new StampDispatchBehavior(context), "Stamps outgoing messages with session ID");
+            configuration.Pipeline.Register(new DiscardMessagesBehavior(context), "Discards messages based on session ID");
+
+            var assemblyScanner = configuration.AssemblyScanner();
+            assemblyScanner.ExcludeAssemblies(Path.GetFileName(typeof(ServiceControlComponentRunner).Assembly.Location));
+        }
     }
 }

src/ServiceControl.AcceptanceTests/TestSupport/HttpClientServiceCollectionExtensions.cs

@@ -8,7 +8,7 @@ namespace ServiceControl.AcceptanceTests.RavenDB.Shared;
 
 static class HttpClientServiceCollectionExtensions
 {
-    public static void OverrideHttpClientDefaults(this IServiceCollection services, Settings settings)
+    public static void AddHttpClientDefaultsOverrides(this IServiceCollection services, Settings settings)
     {
         services.AddKeyedSingleton<Func<HttpMessageHandler>>("Forwarding", (provider, _) => () => provider.GetRequiredService<TestServer>().CreateHandler());
         services.AddSingleton(p => new HttpMessageInvoker(p.GetRequiredKeyedService<Func<HttpMessageHandler>>("Forwarding")()));

src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs

@@ -103,23 +103,7 @@ async Task InitializeServiceControl(ScenarioContext context)
             }
 
             var configuration = new EndpointConfiguration(instanceName);
-
-            configuration.GetSettings().Set("SC.ScenarioContext", context);
-            configuration.GetSettings().Set(context);
-
-            configuration.RegisterComponents(r =>
-            {
-                r.AddSingleton(context.GetType(), context);
-                r.AddSingleton(typeof(ScenarioContext), context);
-            });
-
-            configuration.Pipeline.Register<TraceIncomingBehavior.Registration>();
-            configuration.Pipeline.Register<TraceOutgoingBehavior.Registration>();
-            configuration.Pipeline.Register(new StampDispatchBehavior(context), "Stamps outgoing messages with session ID");
-            configuration.Pipeline.Register(new DiscardMessagesBehavior(context), "Discards messages based on session ID");
-
-            var assemblyScanner = configuration.AssemblyScanner();
-            assemblyScanner.ExcludeAssemblies(Path.GetFileName(typeof(ServiceControlComponentRunner).Assembly.Location));
+            configuration.CustomizeServiceControlEndpointTesting(context);
 
             customConfiguration(configuration);
 
@@ -136,25 +120,7 @@ async Task InitializeServiceControl(ScenarioContext context)
                 });
                 hostBuilder.AddServiceControl(settings, configuration, loggingSettings);
 
-                // Do not register additional test controllers or hosted services here. Instead, in the test that needs them, use (for example):
-                // CustomizeHostBuilder = builder => builder.ConfigureServices((hostContext, services) => services.AddHostedService<SetupNotificationSettings>());
-                hostBuilder.Logging.AddScenarioContextLogging();
-
-                // TODO: the following four lines could go into a AddServiceControlTesting() extension
-                hostBuilder.WebHost.UseTestServer(options => options.BaseAddress = new Uri(settings.RootUrl));
-                hostBuilder.Services.AddSingleton<IHostLifetime, TestServerHostLifetime>();
-
-                // This facilitates receiving the test server anywhere where DI is available
-                hostBuilder.Services.AddSingleton(provider => (TestServer)provider.GetRequiredService<IServer>());
-
-                // By default ASP.NET Core uses entry point assembly to discover controllers from. When running
-                // inside a test runner the runner exe becomes the entry point which obviously has no controllers in it ;)
-                // so we are explicitly registering all necessary application parts.
-                var addControllers = hostBuilder.Services.AddControllers();
-                addControllers.AddApplicationPart(typeof(WebApiHostBuilderExtensions).Assembly);
-                addControllers.AddApplicationPart(typeof(AcceptanceTest).Assembly);
-
-                hostBuilder.Services.OverrideHttpClientDefaults(settings);
+                hostBuilder.AddServiceControlTesting(settings);
 
                 hostBuilderCustomization(hostBuilder);
 

src/ServiceControl.AcceptanceTests/TestSupport/WebApplicationBuilderExtensions.cs

@@ -0,0 +1,35 @@
+namespace ServiceControl.AcceptanceTests.RavenDB.Shared;
+
+using System;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Hosting.Server;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using ServiceBus.Management.Infrastructure.Settings;
+using TestSupport;
+
+static class WebApplicationBuilderExtensions
+{
+    public static void AddServiceControlTesting(this WebApplicationBuilder hostBuilder, Settings settings)
+    {
+        // Do not register additional test controllers or hosted services here. Instead, in the test that needs them, use (for example):
+        // CustomizeHostBuilder = builder => builder.ConfigureServices((hostContext, services) => services.AddHostedService<SetupNotificationSettings>());
+        hostBuilder.Logging.AddScenarioContextLogging();
+
+        hostBuilder.WebHost.UseTestServer(options => options.BaseAddress = new Uri(settings.RootUrl));
+        hostBuilder.Services.AddSingleton<IHostLifetime, TestServerHostLifetime>();
+
+        // This facilitates receiving the test server anywhere where DI is available
+        hostBuilder.Services.AddSingleton(provider => (TestServer)provider.GetRequiredService<IServer>());
+
+        // By default, ASP.NET Core uses the entry point assembly to discover controllers. When running
+        // inside a test runner the runner exe becomes the entry point which obviously has no controllers in it ;)
+        // so we are explicitly registering all necessary application parts.
+        var addControllers = hostBuilder.Services.AddControllers();
+        addControllers.AddApplicationPart(typeof(Settings).Assembly);
+        addControllers.AddApplicationPart(typeof(AcceptanceTest).Assembly);
+
+        hostBuilder.Services.AddHttpClientDefaultsOverrides(settings);
+    }
+}

src/ServiceControl.Audit.AcceptanceTests/AcceptanceTest.cs

@@ -32,8 +32,6 @@ protected AcceptanceTest()
 
         public HttpClient HttpClient => serviceControlRunnerBehavior.HttpClient;
         public JsonSerializerOptions SerializerOptions => serviceControlRunnerBehavior.SerializerOptions;
-
-        // TODO Check why this is necessary and if it can be removed
         protected IServiceProvider ServiceProvider => serviceControlRunnerBehavior.ServiceProvider;
 
         [SetUp]

src/ServiceControl.Audit.AcceptanceTests/TestSupport/EndpointConfigurationExtensions.cs

@@ -0,0 +1,31 @@
+namespace ServiceControl.Audit.AcceptanceTests.TestSupport;
+
+using System.IO;
+using AcceptanceTesting;
+using Microsoft.Extensions.DependencyInjection;
+using NServiceBus;
+using NServiceBus.AcceptanceTesting;
+using NServiceBus.Configuration.AdvancedExtensibility;
+
+static class EndpointConfigurationExtensions
+{
+    public static void CustomizeServiceControlAuditEndpointTesting(this EndpointConfiguration configuration, ScenarioContext context)
+    {
+        configuration.GetSettings().Set("SC.ScenarioContext", context);
+        configuration.GetSettings().Set(context);
+
+        configuration.RegisterComponents(r =>
+        {
+            r.AddSingleton(context.GetType(), context);
+            r.AddSingleton(typeof(ScenarioContext), context);
+        });
+
+        configuration.Pipeline.Register<TraceIncomingBehavior.Registration>();
+        configuration.Pipeline.Register<TraceOutgoingBehavior.Registration>();
+        configuration.Pipeline.Register(new StampDispatchBehavior(context), "Stamps outgoing messages with session ID");
+        configuration.Pipeline.Register(new DiscardMessagesBehavior(context), "Discards messages based on session ID");
+
+        var assemblyScanner = configuration.AssemblyScanner();
+        assemblyScanner.ExcludeAssemblies(Path.GetFileName(typeof(ServiceControlComponentRunner).Assembly.Location));
+    }
+}

src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentBehavior.cs

@@ -11,35 +11,26 @@ namespace ServiceControl.Audit.AcceptanceTests.TestSupport
     using NServiceBus.AcceptanceTesting.Support;
     using ServiceControl.Audit.Infrastructure.Settings;
 
-    class ServiceControlComponentBehavior : IComponentBehavior, IAcceptanceTestInfrastructureProvider
+    class ServiceControlComponentBehavior(
+        ITransportIntegration transportToUse,
+        AcceptanceTestStorageConfiguration persistenceToUse,
+        Action<Settings> setSettings,
+        Action<EndpointConfiguration> customConfiguration,
+        Action<IDictionary<string, string>> setStorageConfiguration,
+        Action<IHostApplicationBuilder> hostBuilderCustomization)
+        : IComponentBehavior, IAcceptanceTestInfrastructureProvider
     {
-        public ServiceControlComponentBehavior(ITransportIntegration transportToUse, AcceptanceTestStorageConfiguration persistenceToUse, Action<Settings> setSettings, Action<EndpointConfiguration> customConfiguration, Action<IDictionary<string, string>> setStorageConfiguration, Action<IHostApplicationBuilder> hostBuilderCustomization)
-        {
-            this.customConfiguration = customConfiguration;
-            this.persistenceToUse = persistenceToUse;
-            this.setSettings = setSettings;
-            this.setStorageConfiguration = setStorageConfiguration;
-            this.hostBuilderCustomization = hostBuilderCustomization;
-            transportIntegration = transportToUse;
-        }
-
         public HttpClient HttpClient => runner.HttpClient;
         public JsonSerializerOptions SerializerOptions => runner.SerializerOptions;
         public IServiceProvider ServiceProvider => runner.ServiceProvider;
 
         public async Task<ComponentRunner> CreateRunner(RunDescriptor run)
         {
-            runner = new ServiceControlComponentRunner(transportIntegration, persistenceToUse, setSettings, customConfiguration, setStorageConfiguration, hostBuilderCustomization);
+            runner = new ServiceControlComponentRunner(transportToUse, persistenceToUse, setSettings, customConfiguration, setStorageConfiguration, hostBuilderCustomization);
             await runner.Initialize(run);
             return runner;
         }
 
-        ITransportIntegration transportIntegration;
-        AcceptanceTestStorageConfiguration persistenceToUse;
-        Action<Settings> setSettings;
-        Action<EndpointConfiguration> customConfiguration;
         ServiceControlComponentRunner runner;
-        Action<IDictionary<string, string>> setStorageConfiguration;
-        Action<IHostApplicationBuilder> hostBuilderCustomization;
     }
 }

src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs

@@ -25,22 +25,15 @@ namespace ServiceControl.Audit.AcceptanceTests.TestSupport
     using NServiceBus.AcceptanceTesting.Support;
     using NServiceBus.Configuration.AdvancedExtensibility;
 
-    public class ServiceControlComponentRunner : ComponentRunner, IAcceptanceTestInfrastructureProvider
+    public class ServiceControlComponentRunner(
+        ITransportIntegration transportToUse,
+        AcceptanceTestStorageConfiguration persistenceToUse,
+        Action<Settings> setSettings,
+        Action<EndpointConfiguration> customConfiguration,
+        Action<IDictionary<string, string>> setStorageConfiguration,
+        Action<IHostApplicationBuilder> hostBuilderCustomization)
+        : ComponentRunner, IAcceptanceTestInfrastructureProvider
     {
-        public ServiceControlComponentRunner(ITransportIntegration transportToUse,
-            AcceptanceTestStorageConfiguration persistenceToUse, Action<Settings> setSettings,
-            Action<EndpointConfiguration> customConfiguration,
-            Action<IDictionary<string, string>> setStorageConfiguration,
-            Action<IHostApplicationBuilder> hostBuilderCustomization)
-        {
-            this.transportToUse = transportToUse;
-            this.persistenceToUse = persistenceToUse;
-            this.customConfiguration = customConfiguration;
-            this.setStorageConfiguration = setStorageConfiguration;
-            this.setSettings = setSettings;
-            this.hostBuilderCustomization = hostBuilderCustomization;
-        }
-
         public override string Name { get; } = $"{nameof(ServiceControlComponentRunner)}";
         public HttpClient HttpClient { get; private set; }
         public JsonSerializerOptions SerializerOptions => Infrastructure.WebApi.SerializerOptions.Default;
@@ -109,23 +102,7 @@ async Task InitializeServiceControl(ScenarioContext context)
             }
 
             var configuration = new EndpointConfiguration(instanceName);
-
-            configuration.GetSettings().Set("SC.ScenarioContext", context);
-            configuration.GetSettings().Set(context);
-
-            configuration.RegisterComponents(r =>
-            {
-                r.AddSingleton(context.GetType(), context);
-                r.AddSingleton(typeof(ScenarioContext), context);
-            });
-
-            configuration.Pipeline.Register<TraceIncomingBehavior.Registration>();
-            configuration.Pipeline.Register<TraceOutgoingBehavior.Registration>();
-            configuration.Pipeline.Register(new StampDispatchBehavior(context), "Stamps outgoing messages with session ID");
-            configuration.Pipeline.Register(new DiscardMessagesBehavior(context), "Discards messages based on session ID");
-
-            var assemblyScanner = configuration.AssemblyScanner();
-            assemblyScanner.ExcludeAssemblies(Path.GetFileName(typeof(ServiceControlComponentRunner).Assembly.Location));
+            configuration.CustomizeServiceControlAuditEndpointTesting(context);
 
             customConfiguration(configuration);
 
@@ -153,28 +130,13 @@ async Task InitializeServiceControl(ScenarioContext context)
                     return criticalErrorContext.Stop(cancellationToken);
                 }, settings, configuration, loggingSettings);
 
-                // Do not register additional test controllers or hosted services here. Instead, in the test that needs them, use (for example):
-                // CustomizeHostBuilder = builder => builder.ConfigureServices((hostContext, services) => services.AddHostedService<SetupNotificationSettings>());
-                hostBuilder.Logging.AddScenarioContextLogging();
-
-                // TODO: the following four lines could go into a AddServiceControlAuditTesting() extension
-                hostBuilder.WebHost.UseTestServer(options => options.BaseAddress = new Uri(settings.RootUrl));
-                // This facilitates receiving the test server anywhere where DI is available
-                hostBuilder.Services.AddSingleton(provider => (TestServer)provider.GetRequiredService<IServer>());
-
-                // By default ASP.NET Core uses entry point assembly to discover controllers from. When running
-                // inside a test runner the runner exe becomes the entry point which obviously has no controllers in it ;)
-                // so we are explicitly registering all necessary application parts.
-                var addControllers = hostBuilder.Services.AddControllers();
-                addControllers.AddApplicationPart(typeof(WebApiHostBuilderExtensions).Assembly);
-                addControllers.AddApplicationPart(typeof(FailedAuditsController).Assembly);
+                hostBuilder.AddServiceControlAuditTesting(settings);
 
                 hostBuilderCustomization(hostBuilder);
 
                 host = hostBuilder.Build();
                 host.UseServiceControlAudit();
                 await host.StartAsync();
-                // TODO We can probably remove this by switching over to the hostBuilderCustomization
                 ServiceProvider = host.Services;
                 InstanceTestServer = host.GetTestServer();
                 HttpClient = InstanceTestServer.CreateClient();
@@ -191,12 +153,6 @@ public override async Task Stop(CancellationToken cancellationToken = default)
             }
         }
 
-        ITransportIntegration transportToUse;
-        AcceptanceTestStorageConfiguration persistenceToUse;
-        Action<Settings> setSettings;
-        Action<EndpointConfiguration> customConfiguration;
-        Action<IDictionary<string, string>> setStorageConfiguration;
-        Action<IHostApplicationBuilder> hostBuilderCustomization;
         string instanceName = Settings.DEFAULT_SERVICE_NAME;
         WebApplication host;
         Settings settings;

src/ServiceControl.Audit.AcceptanceTests/TestSupport/WebApplicationBuilderExtensions.cs

@@ -0,0 +1,31 @@
+namespace ServiceControl.Audit.AcceptanceTests.TestSupport;
+
+using System;
+using Auditing;
+using Infrastructure.Settings;
+using Infrastructure.WebApi;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Hosting.Server;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.DependencyInjection;
+
+static class WebApplicationBuilderExtensions
+{
+    public static void AddServiceControlAuditTesting(this WebApplicationBuilder hostBuilder, Settings settings)
+    {
+        // Do not register additional test controllers or hosted services here. Instead, in the test that needs them, use (for example):
+        // CustomizeHostBuilder = builder => builder.ConfigureServices((hostContext, services) => services.AddHostedService<SetupNotificationSettings>());
+        hostBuilder.Logging.AddScenarioContextLogging();
+
+        hostBuilder.WebHost.UseTestServer(options => options.BaseAddress = new Uri(settings.RootUrl));
+        // This facilitates receiving the test server anywhere where DI is available
+        hostBuilder.Services.AddSingleton(provider => (TestServer)provider.GetRequiredService<IServer>());
+
+        // By default, ASP.NET Core uses the entry point assembly to discover controllers. When running
+        // inside a test runner the runner exe becomes the entry point which obviously has no controllers in it ;)
+        // so we are explicitly registering all necessary application parts.
+        var addControllers = hostBuilder.Services.AddControllers();
+        addControllers.AddApplicationPart(typeof(Settings).Assembly);
+        addControllers.AddApplicationPart(typeof(AcceptanceTest).Assembly);
+    }
+}

src/ServiceControl.Audit/Infrastructure/WebApi/WebApiHostBuilderExtensions.cs → src/ServiceControl.Audit/Infrastructure/WebApi/WebApplicationBuilderExtensions.cs

@@ -4,7 +4,7 @@
     using Microsoft.AspNetCore.Hosting;
     using Microsoft.Extensions.DependencyInjection;
 
-    static class WebApiHostBuilderExtensions
+    static class WebApplicationBuilderExtensions
     {
         public static void AddWebApi(this WebApplicationBuilder builder, string rootUrl)
         {