Skip to content

Use InMemoryEventBus as scoped and use InMemoryEventBus in integration tests #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Use InMemoryEventBus as scoped and use InMemoryEventBus in integration tests #99

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9446fbf
fix: register mediator as scoped
anddrzejb Mar 19, 2024
daabec0
fix: integration tests to use InMemoryEventBus
anddrzejb Mar 19, 2024
7536d50
refactor: rename file to match class name
anddrzejb Mar 19, 2024
d68a0ac
refactor: move few namespaces to global using and make NotificationDe…
anddrzejb Mar 22, 2024
eae74a0
fix: NotificationDecorator bug causing NotificationHandlers to be run…
anddrzejb Mar 22, 2024
c023fb8
Merge branch 'main' into fix/use-mediator-as-scoped-and-use-inmemorye…
anddrzejb Mar 22, 2024
cf501d3
Merge branch 'main' into fix/use-mediator-as-scoped-and-use-inmemorye…
anddrzejb Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...ture/Src/Fitnet.IntegrationTests/Common/Events/EventBus/InMemory/InMemoryEventBusTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ public sealed class InMemoryEventBusTests(
internal async Task Given_valid_event_published_Then_event_should_be_consumed()
{
// Arrange
var eventBus = _applicationInMemory.Services.GetRequiredService<IEventBus>();
var serviceScope = _applicationInMemory.Services.CreateScope();
var eventBus = serviceScope.ServiceProvider.GetRequiredService<IEventBus>();
var fakeEvent = FakeEvent.Create();

// Act
Expand Down
21 changes: 21 additions & 0 deletions ...ture/Src/Fitnet.IntegrationTests/Common/Events/EventBus/InMemory/NotificationDecorator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace EvolutionaryArchitecture.Fitnet.IntegrationTests.Common.Events.EventBus.InMemory;

using Fitnet.Common.Events;
using Fitnet.Common.Events.EventBus;
using MediatR;

internal class NotificationDecorator<TNotification>(IEventBus eventBus, INotificationHandler<TNotification>? innerHandler)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It introduces quite a lot of boilerplate code when adding it in TestEngine. Still, we agree that it would make sense to use MediatR but maybe with a slightly different approach. What do you think to use MediatR but fake the method responsible for publishing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that there is a test Given_valid_mark_pass_as_expired_request_Then_should_return_no_content where:

  1. The pass is registered in the notification handler (ContractSignedEvent is published and needs to be handled by ContractSignedEventHandler, where the pass is persisted) -> here you had this intricate IntegrationEventHandlerScope flow in previous approach
  2. The expiration endpoint is called which is served IMediator based InMemoryEventBus. The PassExpiredEvent is using the bus.
  3. PassExpiredEventHandler is marking the pass as expired in the database. Also publishes OfferPreparedEvent.

If I am going to fake PublishAsync instead of using built-in MediatR, I am afraid that:
Ad.1. The ContractSignedEventHandler won't be called
Ad.3. The PassExpiredEventHandler won't be called
I believe there are more tests like that.

On top of that, if Mediator won't run its internals, then the bug #96 that sparked this change will hide again, as it surfaced because Mediator actually executed the Publish method.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anddrzejb to be honest I wouldn't be afraid that it won't be called - this is an internal mechanism of MediatR that should not be tested. IMO it would be enough to know that the event was published and that's it.

@kamilbaczek can you add your opinion on it as well? I do not want to be "that one badass who destroys dreams" :D

If you disagree, there is still a chance to simplify the above code:

internal static WebApplicationFactory<T> WithFakeEventBus<T>(this WebApplicationFactory<T> webApplicationFactory,
        IEventBus eventBusMock)
        where T : class => webApplicationFactory.WithWebHostBuilder(builder =>
    {
        builder.ConfigureTestServices(services =>
        {
            ReplaceNotificationHandlers(eventBusMock, services);
        });
    });

private static void ReplaceNotificationHandlers(IEventBus eventBusMock, IServiceCollection services)
    {
        var notificationHandlerTypes = GetNotificationHandlerTypes(services);

        foreach (var handlerType in notificationHandlerTypes)
        {
            var decoratorType = typeof(NotificationDecorator<>).MakeGenericType(handlerType.GetGenericArguments()[0]);

            services.AddTransient(handlerType, serviceProvider =>
            {
                var innerHandler = serviceProvider.GetService(handlerType);
                var decorator = Activator.CreateInstance(decoratorType, eventBusMock, innerHandler);

                return decorator!;
            });
        }
    }

    private static List<Type> GetNotificationHandlerTypes(IServiceCollection services) => services
            .Where(descriptor => descriptor.ServiceType.IsGenericType &&
                                 descriptor.ServiceType.GetGenericTypeDefinition() == typeof(INotificationHandler<>))
            .Select(descriptor => descriptor.ServiceType)
            .ToList();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more comment from my side - in such case it would be acceptable to introduce the E2E test that would check the business process. Unfortunately (on purpose) we do not have it in this project but unfortunately we need to decide for some trade-offs because of that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me first address the example test I was mentioning: Given_valid_mark_pass_as_expired_request_Then_should_return_no_content.

  1. This test is expecting a PassExpiredEvent to be published.
  2. Based on the code, it should be published once api/passes/{id} endpoint is called - given the pass exists. Because this is an integration test, then I presume it should wire the application in such a way, so the event is indeed published in the endpoint handler.
  3. For the endpoint to find the pass, it needs to exist in database. The arrange section of the test is responsible for that.
    a. Publishes ContractSignedEvent
    b. The handler is resolved based on the event and called.
    c. The handler creates the pass in the database and publishes PassRegisteredEvent. We do not want to reallyy publish it, this is not part of the test, after all we are still in the arrange section.
  4. The pass id is retrieved from the database so it can be used later in the test (still in the arrange section of the test)
  5. The endpoint api/passes/{id}, where {id} is the value retrieved in point 4 (act)
  6. The mock is checked if the event PassExpiredEvent was indeed called (assert).

Here we have 2 scenarios - I: use mediator or II: fake it.
I. Mediator. The tricky part is to catch the 2nd event published in 3c. Otherwise, decorator does its thing. This is where the fragility of my solution comes out - what if we have a scenario when we need to stop 3rd event, or 4th? Currently, such scenario is not handled, unless manually issued in the arrange/act section. But also seems that at this point is not needed. But it is not handled by II either.
II. Fake the mediator. Then we have to deal with the 3b. We can easily do it using fake, but it adds complexity to the solution by introducing IntegrationEventHandlerScope, which under the hood seems to be doing what mediator does. Without its shortcomings. Which is not necessarily good given that the test did not discover the bug.

Here is where we come to the bottom of the issue. Integration tests should be testing all the components that are involved in the testing scenario. When they are not tested, then bugs may creep in (I rest my case?). The argument that Mediator is an internal mechanism and should not be tested - if it was a unit test I wholeheartedly agree. But in this case it is not Mediator being tested but its correct/incorrect integration.

Regarding the proposed change - it goes into infinite loop. When mediator is trying to resolve INotification

  • it goes into the delegate passed in services.AddTransient. Delegate Inside is trying to get from the services the handlerType, that is the same type as the mediator was trying to get. So...
    • it goes into the delegate passed in services.AddTransient. Delegate Inside is trying to get from the services the handlerType, that is the same type as the mediator was trying to get. So...
      • it goes into the delegate passed in services.AddTransient. Delegate Inside is trying to get from the services the handlerType, that is the same type as the mediator was trying to get. So...
        (....) surprisingly I never run into stack overflow.

"that one badass who destroys dreams"

No worries, I am not that fragile 😄

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss this together on the next sync

: INotificationHandler<TNotification>
where TNotification : INotification
{
public async Task Handle(TNotification notification, CancellationToken cancellationToken)
{
var @event = notification as IIntegrationEvent;
await eventBus.PublishAsync(@event!, cancellationToken);

if (innerHandler is not null)
{
await innerHandler.Handle(notification, cancellationToken);
}
}
}
66 changes: 64 additions & 2 deletions ...re/Src/Fitnet.IntegrationTests/Common/TestEngine/Configuration/ConfigurationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
namespace EvolutionaryArchitecture.Fitnet.IntegrationTests.Common.TestEngine.Configuration;

using System.Reflection;
using Events.EventBus.InMemory;
using EvolutionaryArchitecture.Fitnet.Common.Events.EventBus;
using EvolutionaryArchitecture.Fitnet.Common.Events.EventBus.InMemory;
using MediatR;
using Microsoft.AspNetCore.Mvc.Testing;
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.Time.Testing;
Expand Down Expand Up @@ -41,10 +43,70 @@ internal static WebApplicationFactory<T> SetFakeSystemClock<T>(this WebApplicati
internal static WebApplicationFactory<T> WithFakeEventBus<T>(this WebApplicationFactory<T> webApplicationFactory, IEventBus eventBusMock)
where T : class =>
webApplicationFactory.WithWebHostBuilder(webHostBuilder => webHostBuilder.ConfigureTestServices(services =>
services.AddSingleton(eventBusMock)));
{
AddNotificationDecorator(eventBusMock, services, true);
}));

internal static WebApplicationFactory<T> WithoutEventHandlers<T>(this WebApplicationFactory<T> webApplicationFactory, IEventBus eventBusMock)
where T : class =>
webApplicationFactory.WithWebHostBuilder(webHostBuilder => webHostBuilder.ConfigureTestServices(services =>
{
AddNotificationDecorator(eventBusMock, services, false);
}));

internal static WebApplicationFactory<T> WithFakeConsumers<T>(this WebApplicationFactory<T> webApplicationFactory)
where T : class =>
webApplicationFactory.WithWebHostBuilder(webHostBuilder => webHostBuilder.ConfigureTestServices(services =>
services.AddInMemoryEventBus(Assembly.GetExecutingAssembly())));
{
services.AddInMemoryEventBus(Assembly.GetExecutingAssembly());
var decorator = services.FirstOrDefault(s => s.ImplementationType == typeof(NotificationDecorator<>));
if (decorator is not null)
{
services.Remove(decorator);
}
}));

private static void AddNotificationDecorator(IEventBus eventBusMock, IServiceCollection services, bool hasSingleEventHandling)
{
var notificationHandlerServices = services.Where(s =>
s.ServiceType.IsGenericType &&
s.ServiceType.GetGenericTypeDefinition() == typeof(INotificationHandler<>))
.ToList();

foreach (var notificationHandlerService in notificationHandlerServices)
{
var serviceType = notificationHandlerService.ServiceType;
var implementationType = notificationHandlerService.ImplementationType;

services.AddTransient(serviceType, serviceProvider =>
{
var notificationHandler = hasSingleEventHandling
? NotificationHandlerWithFakeEventBus(eventBusMock, implementationType, serviceProvider)
: null;

var decoratorType = typeof(NotificationDecorator<>).MakeGenericType(serviceType.GenericTypeArguments[0]);
return Activator.CreateInstance(decoratorType, eventBusMock, notificationHandler)!;
});
}
}

private static object NotificationHandlerWithFakeEventBus(IEventBus eventBusMock, Type? implementationType,
IServiceProvider serviceProvider)
{
var constructor = implementationType!.GetConstructors().FirstOrDefault()!;
var parameters = constructor.GetParameters();
var handlerDependencies = new object[parameters.Length];
for (var i = 0; i < parameters.Length; i++)
{
if (parameters[i].ParameterType == typeof(IEventBus))
{
handlerDependencies[i] = eventBusMock;
continue;
}
handlerDependencies[i] = serviceProvider.GetService(parameters[i]!.ParameterType)!;
}

var notificationHandler = Activator.CreateInstance(implementationType, handlerDependencies)!;
return notificationHandler;
}
}
2 changes: 1 addition & 1 deletion ...tial-architecture/Src/Fitnet.IntegrationTests/Contracts/SignContract/SignContractTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public sealed class SignContractTests : IClassFixture<WebApplicationFactory<Prog
public SignContractTests(WebApplicationFactory<Program> applicationInMemoryFactory,
DatabaseContainer database) =>
_applicationHttpClient = applicationInMemoryFactory
.WithFakeEventBus(_fakeEventBus)
.WithoutEventHandlers(_fakeEventBus)
.WithContainerDatabaseConfigured(database.ConnectionString!)
.CreateClient();

Expand Down
13 changes: 4 additions & 9 deletions ...er-1-initial-architecture/Src/Fitnet.IntegrationTests/Offers/Prepare/PrepareOfferTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
namespace EvolutionaryArchitecture.Fitnet.IntegrationTests.Offers.Prepare;

using Common.TestEngine.Configuration;
using Common.TestEngine.IntegrationEvents.Handlers;
using Fitnet.Offers.Prepare;
using Fitnet.Passes.MarkPassAsExpired.Events;
using EvolutionaryArchitecture.Fitnet.Common.Events.EventBus;


public sealed class PrepareOfferTests : IClassFixture<WebApplicationFactory<Program>>,
IClassFixture<DatabaseContainer>
{
Expand All @@ -27,18 +24,16 @@ public PrepareOfferTests(WebApplicationFactory<Program> applicationInMemoryFacto
internal async Task Given_pass_expired_event_published_Then_new_offer_should_be_prepared()
{
// Arrange
using var integrationEventHandlerScope =
new IntegrationEventHandlerScope<PassExpiredEvent>(_applicationInMemory);
var integrationEventHandler = integrationEventHandlerScope.IntegrationEventHandler;
using var serviceScope = _applicationInMemory.Services.CreateScope();
var eventBus = serviceScope.ServiceProvider.GetRequiredService<IEventBus>();
var @event = PassExpiredEventFaker.CreateValid();

// Act
await integrationEventHandler.Handle(@event, CancellationToken.None);
await eventBus.PublishAsync(@event);

// Assert
EnsureThatOfferPreparedEventWasPublished();
}

private void EnsureThatOfferPreparedEventWasPublished() => _fakeEventBus.Received(1)
.PublishAsync(Arg.Any<OfferPrepareEvent>(), Arg.Any<CancellationToken>());
}
}
7 changes: 3 additions & 4 deletions ...chitecture/Src/Fitnet.IntegrationTests/Passes/MarkPassAsExpired/MarkPassAsExpiredTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ namespace EvolutionaryArchitecture.Fitnet.IntegrationTests.Passes.MarkPassAsExpi
using Fitnet.Passes;
using RegisterPass;
using Common.TestEngine.Configuration;
using Common.TestEngine.IntegrationEvents.Handlers;
using EvolutionaryArchitecture.Fitnet.Common.Events.EventBus;
using Fitnet.Contracts.SignContract.Events;
using Fitnet.Passes.GetAllPasses;
Expand Down Expand Up @@ -74,9 +73,9 @@ internal async Task Given_mark_pass_as_expired_request_with_not_existing_id_Then
private async Task<ContractSignedEvent> RegisterPass()
{
var @event = ContractSignedEventFaker.Create();
using var integrationEventHandlerScope =
new IntegrationEventHandlerScope<ContractSignedEvent>(_applicationInMemoryFactory);
await integrationEventHandlerScope.Consume(@event);
using var serviceScope = _applicationInMemoryFactory.Services.CreateScope();
var eventBus = serviceScope.ServiceProvider.GetRequiredService<IEventBus>();
await eventBus.PublishAsync(@event);

return @event;
}
Expand Down
8 changes: 3 additions & 5 deletions ...initial-architecture/Src/Fitnet.IntegrationTests/Passes/RegisterPass/RegisterPassTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
namespace EvolutionaryArchitecture.Fitnet.IntegrationTests.Passes.RegisterPass;

using Common.TestEngine.Configuration;
using Common.TestEngine.IntegrationEvents.Handlers;
using Fitnet.Contracts.SignContract.Events;
using Fitnet.Passes.RegisterPass.Events;
using EvolutionaryArchitecture.Fitnet.Common.Events.EventBus;

Expand All @@ -25,12 +23,12 @@ public RegisterPassTests(WebApplicationFactory<Program> applicationInMemoryFacto
internal async Task Given_contract_signed_event_Then_should_register_pass()
{
// Arrange
using var integrationEventHandlerScope =
new IntegrationEventHandlerScope<ContractSignedEvent>(_applicationInMemory);
using var serviceScope = _applicationInMemory.Services.CreateScope();
var eventBus = serviceScope.ServiceProvider.GetRequiredService<IEventBus>();
var @event = ContractSignedEventFaker.Create();

// Act
await integrationEventHandlerScope.Consume(@event);
await eventBus.PublishAsync(@event);

// Assert
EnsureThatPassRegisteredEventWasPublished();
Expand Down
12 changes: 6 additions & 6 deletions ...tionTests/Reports/GenerateNewPassesPerMonthReport/GenerateNewPassesPerMonthReportTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
namespace EvolutionaryArchitecture.Fitnet.IntegrationTests.Reports.GenerateNewPassesPerMonthReport;

using Common.TestEngine.Configuration;
using Common.TestEngine.IntegrationEvents.Handlers;
using Fitnet.Contracts.SignContract.Events;
using Fitnet.Common.Events.EventBus;
using Fitnet.Reports;
using Fitnet.Reports.GenerateNewPassesRegistrationsPerMonthReport.Dtos;
using Passes.RegisterPass;
Expand Down Expand Up @@ -51,10 +50,11 @@ private async Task RegisterPasses(List<PassRegistrationDateRange> reportTestData

private async Task RegisterPass(DateTimeOffset from, DateTimeOffset to)
{
using var integrationEventHandlerScope =
new IntegrationEventHandlerScope<ContractSignedEvent>(_applicationInMemoryFactory);
var integrationEventHandler = integrationEventHandlerScope.IntegrationEventHandler;
using var serviceScope = _applicationInMemoryFactory.Services.CreateScope();
var eventBus = serviceScope.ServiceProvider.GetRequiredService<IEventBus>();
var @event = ContractSignedEventFaker.Create(from, to);
await integrationEventHandler.Handle(@event, CancellationToken.None);

// Act
await eventBus.PublishAsync(@event);
}
}
2 changes: 1 addition & 1 deletion ...initial-architecture/Src/Fitnet/Common/Events/EventBus/InMemory/InMemoryEventBusModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ internal static class InMemoryEventBusModule
{
internal static IServiceCollection AddInMemoryEventBus(this IServiceCollection services, Assembly assembly)
{
services.AddSingleton<IEventBus, InMemoryEventBus>();
services.AddScoped<IEventBus, InMemoryEventBus>();
services.AddMediatR(configuration => configuration.RegisterServicesFromAssembly(assembly));

return services;
Expand Down
0 ...t/Passes/RegisterPass/RegisterEndpoint.cs → ...egisterPass/ContractSignedEventHandler.cs
View file
Open in desktop
File renamed without changes.