From 62c3e0312d878fde7c5ada7025159e8521321ae8 Mon Sep 17 00:00:00 2001 From: FrostyApeOne <78855469+FrostyApeOne@users.noreply.github.com> Date: Wed, 18 Sep 2024 09:32:08 +0100 Subject: [PATCH] Improving unit tests (#607) --- .../GetMemberOfParliamentByConstituencies.cs | 8 ++--- .../Dfe.Academies.Application.Tests.csproj | 6 ++-- ...rliamentByConstituencyQueryHandlerTests.cs | 26 +++++++++++++---- ...amentsByConstituenciesQueryHandlerTests.cs | 27 +++++++++++++---- ...lPersonsAssociatedWithAcademyByUrnTests.cs | 29 +++++++++++++++---- .../Customizations/AutoMapperCustomization.cs | 3 +- .../Models/AcademyGovernanceCustomization.cs | 22 +++++++++----- .../Models/MemberOfParliamentCustomization.cs | 25 +++++++++++----- .../OmitCircularReferenceCustomization.cs | 15 ++++++++++ .../Dfe.Academies.Testing.Common.csproj | 3 ++ 10 files changed, 123 insertions(+), 41 deletions(-) create mode 100644 Tests/Dfe.Academies.Testing.Common/Customizations/OmitCircularReferenceCustomization.cs diff --git a/Dfe.Academies.Application/Constituencies/Queries/GetMemberOfParliamentByConstituencies/GetMemberOfParliamentByConstituencies.cs b/Dfe.Academies.Application/Constituencies/Queries/GetMemberOfParliamentByConstituencies/GetMemberOfParliamentByConstituencies.cs index 4c6d362f8..6512845a7 100644 --- a/Dfe.Academies.Application/Constituencies/Queries/GetMemberOfParliamentByConstituencies/GetMemberOfParliamentByConstituencies.cs +++ b/Dfe.Academies.Application/Constituencies/Queries/GetMemberOfParliamentByConstituencies/GetMemberOfParliamentByConstituencies.cs @@ -1,11 +1,11 @@ using AutoMapper; -using MediatR; -using Dfe.Academies.Application.Common.Models; -using Dfe.Academies.Utils.Caching; using AutoMapper.QueryableExtensions; -using Microsoft.EntityFrameworkCore; +using Dfe.Academies.Application.Common.Models; using Dfe.Academies.Domain.Interfaces.Caching; using Dfe.Academies.Domain.Interfaces.Repositories; +using Dfe.Academies.Utils.Caching; +using MediatR; +using Microsoft.EntityFrameworkCore; namespace Dfe.Academies.Application.Constituencies.Queries.GetMemberOfParliamentByConstituencies { diff --git a/Tests/Dfe.Academies.Application.Tests/Dfe.Academies.Application.Tests.csproj b/Tests/Dfe.Academies.Application.Tests/Dfe.Academies.Application.Tests.csproj index 7210b9b4c..86828612e 100644 --- a/Tests/Dfe.Academies.Application.Tests/Dfe.Academies.Application.Tests.csproj +++ b/Tests/Dfe.Academies.Application.Tests/Dfe.Academies.Application.Tests.csproj @@ -10,11 +10,9 @@ - + - - - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentByConstituencyQueryHandlerTests.cs b/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentByConstituencyQueryHandlerTests.cs index e6764a182..b109b4e3e 100644 --- a/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentByConstituencyQueryHandlerTests.cs +++ b/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentByConstituencyQueryHandlerTests.cs @@ -1,4 +1,5 @@ -using AutoFixture.Xunit2; +using AutoFixture; +using AutoFixture.Xunit2; using Dfe.Academies.Application.Common.Models; using Dfe.Academies.Application.Constituencies.Queries.GetMemberOfParliamentByConstituency; using Dfe.Academies.Domain.Interfaces.Caching; @@ -25,15 +26,30 @@ public async Task Handle_ShouldReturnMemberOfParliament_WhenConstituencyExists( GetMemberOfParliamentByConstituencyQueryHandler handler, GetMemberOfParliamentByConstituencyQuery query, Domain.Constituencies.Constituency constituency, - MemberOfParliament expectedMp) + IFixture fixture) { // Arrange + var expectedMp = fixture.Customize(new MemberOfParliamentCustomization() + { + FirstName = constituency.NameDetails.NameListAs.Split(",")[1].Trim(), + LastName = constituency.NameDetails.NameListAs.Split(",")[0].Trim(), + ConstituencyName = constituency.ConstituencyName, + }).Create(); + var cacheKey = $"MemberOfParliament_{CacheKeyHelper.GenerateHashedCacheKey(query.ConstituencyName)}"; + mockConstituencyRepository.GetMemberOfParliamentByConstituencyAsync(query.ConstituencyName, default) .Returns(constituency); - mockCacheService.GetOrAddAsync(cacheKey, Arg.Any>>(), Arg.Any()) - .Returns(expectedMp); + mockCacheService.GetOrAddAsync( + cacheKey, + Arg.Any>>(), + Arg.Any()) + .Returns(callInfo => + { + var callback = callInfo.ArgAt>>(1); + return callback(); + }); // Act var result = await handler.Handle(query, default); @@ -44,7 +60,7 @@ public async Task Handle_ShouldReturnMemberOfParliament_WhenConstituencyExists( Assert.Equal(expectedMp.LastName, result.LastName); Assert.Equal(expectedMp.ConstituencyName, result.ConstituencyName); - await mockCacheService.Received(1).GetOrAddAsync(cacheKey, Arg.Any>>(), nameof(GetMemberOfParliamentByConstituencyQueryHandler)); + await mockConstituencyRepository.Received(1).GetMemberOfParliamentByConstituencyAsync(query.ConstituencyName, default); } } } diff --git a/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentsByConstituenciesQueryHandlerTests.cs b/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentsByConstituenciesQueryHandlerTests.cs index c8c36e6d9..6733f5955 100644 --- a/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentsByConstituenciesQueryHandlerTests.cs +++ b/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Constituency/GetMemberOfParliamentsByConstituenciesQueryHandlerTests.cs @@ -1,4 +1,5 @@ -using AutoFixture.Xunit2; +using AutoFixture; +using AutoFixture.Xunit2; using Dfe.Academies.Application.Common.Models; using Dfe.Academies.Application.Constituencies.Queries.GetMemberOfParliamentByConstituencies; using Dfe.Academies.Domain.Interfaces.Caching; @@ -8,9 +9,9 @@ using Dfe.Academies.Testing.Common.Customizations.Entities; using Dfe.Academies.Testing.Common.Customizations.Models; using Dfe.Academies.Utils.Caching; +using MockQueryable; using NSubstitute; - namespace Dfe.Academies.Application.Tests.QueryHandlers.Constituency { public class GetMemberOfParliamentByConstituenciesQueryHandlerTests @@ -26,16 +27,30 @@ public async Task Handle_ShouldReturnMemberOfParliament_WhenConstituencyExists( GetMembersOfParliamentByConstituenciesQueryHandler handler, GetMembersOfParliamentByConstituenciesQuery query, List constituencies, - List expectedMps) + IFixture fixture) { // Arrange + var expectedMps = constituencies.Select(constituency => + fixture.Customize(new MemberOfParliamentCustomization() + { + FirstName = constituency.NameDetails.NameListAs.Split(",")[1].Trim(), + LastName = constituency.NameDetails.NameListAs.Split(",")[0].Trim(), + ConstituencyName = constituency.ConstituencyName, + }).Create()).ToList(); + var cacheKey = $"MemberOfParliament_{CacheKeyHelper.GenerateHashedCacheKey(query.ConstituencyNames)}"; + var mock = constituencies.BuildMock(); + mockConstituencyRepository.GetMembersOfParliamentByConstituenciesQueryable(query.ConstituencyNames) - .Returns(constituencies.AsQueryable()); + .Returns(mock); mockCacheService.GetOrAddAsync(cacheKey, Arg.Any>>>(), Arg.Any()) - .Returns(expectedMps); + .Returns(callInfo => + { + var callback = callInfo.ArgAt>>>(1); + return callback(); + }); // Act var result = await handler.Handle(query, default); @@ -50,7 +65,7 @@ public async Task Handle_ShouldReturnMemberOfParliament_WhenConstituencyExists( Assert.Equal(expectedMps[i].ConstituencyName, result[i].ConstituencyName); } - await mockCacheService.Received(1).GetOrAddAsync(cacheKey, Arg.Any>>>(), nameof(GetMembersOfParliamentByConstituenciesQueryHandler)); + mockConstituencyRepository.Received(1).GetMembersOfParliamentByConstituenciesQueryable(query.ConstituencyNames); } } } diff --git a/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Establishment/GetAllPersonsAssociatedWithAcademyByUrnTests.cs b/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Establishment/GetAllPersonsAssociatedWithAcademyByUrnTests.cs index 2d9d4d168..1531576e1 100644 --- a/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Establishment/GetAllPersonsAssociatedWithAcademyByUrnTests.cs +++ b/Tests/Dfe.Academies.Application.Tests/QueryHandlers/Establishment/GetAllPersonsAssociatedWithAcademyByUrnTests.cs @@ -1,4 +1,5 @@ -using AutoFixture.Xunit2; +using AutoFixture; +using AutoFixture.Xunit2; using Dfe.Academies.Application.Common.Interfaces; using Dfe.Academies.Application.Common.Models; using Dfe.Academies.Application.Establishment.Queries.GetAllPersonsAssociatedWithAcademyByUrn; @@ -8,6 +9,7 @@ using Dfe.Academies.Testing.Common.Customizations; using Dfe.Academies.Testing.Common.Customizations.Models; using Dfe.Academies.Utils.Caching; +using MockQueryable; using NSubstitute; namespace Dfe.Academies.Application.Tests.QueryHandlers.Establishment @@ -16,6 +18,7 @@ public class GetAllPersonsAssociatedWithAcademyByUrnQueryHandlerTests { [Theory] [CustomAutoData( + typeof(OmitCircularReferenceCustomization), typeof(AcademyGovernanceCustomization), typeof(AcademyGovernanceQueryModelCustomization), typeof(AutoMapperCustomization))] @@ -24,17 +27,31 @@ public async Task Handle_ShouldReturnPersonsAssociatedWithAcademy_WhenUrnExists( [Frozen] ICacheService mockCacheService, GetAllPersonsAssociatedWithAcademyByUrnQueryHandler handler, GetAllPersonsAssociatedWithAcademyByUrnQuery query, - List expectedGovernances, - IQueryable governanceQueryModels) + List governanceQueryModels, + IFixture fixture) + { // Arrange + var expectedGovernances = governanceQueryModels.Select(governance => + fixture.Customize(new AcademyGovernanceCustomization + { + FirstName = governance?.EducationEstablishmentGovernance?.Forename1, + LastName = governance?.EducationEstablishmentGovernance?.Surname, + }).Create()).ToList(); + var cacheKey = $"PersonsAssociatedWithAcademy_{CacheKeyHelper.GenerateHashedCacheKey(query.Urn.ToString())}"; + var mock = governanceQueryModels.BuildMock(); + mockEstablishmentQueryService.GetPersonsAssociatedWithAcademyByUrn(query.Urn) - .Returns(governanceQueryModels); + .Returns(mock); - mockCacheService.GetOrAddAsync(cacheKey, Arg.Any?>>>(), Arg.Any()) - .Returns(expectedGovernances); + mockCacheService.GetOrAddAsync(cacheKey, Arg.Any>>>(), Arg.Any()) + .Returns(callInfo => + { + var callback = callInfo.ArgAt>>>(1); + return callback(); + }); // Act var result = await handler.Handle(query, default); diff --git a/Tests/Dfe.Academies.Testing.Common/Customizations/AutoMapperCustomization.cs b/Tests/Dfe.Academies.Testing.Common/Customizations/AutoMapperCustomization.cs index 5903a5db9..f77c52613 100644 --- a/Tests/Dfe.Academies.Testing.Common/Customizations/AutoMapperCustomization.cs +++ b/Tests/Dfe.Academies.Testing.Common/Customizations/AutoMapperCustomization.cs @@ -1,5 +1,6 @@ using AutoFixture; using AutoMapper; +using Dfe.Academies.Application.MappingProfiles; using System.Reflection; namespace Dfe.Academies.Testing.Common.Customizations @@ -10,7 +11,7 @@ public void Customize(IFixture fixture) { fixture.Customize(composer => composer.FromFactory(() => { - var profiles = Assembly.GetExecutingAssembly() + var profiles = typeof(ConstituencyProfile).Assembly .GetTypes() .Where(t => typeof(Profile).IsAssignableFrom(t) && !t.IsAbstract) .ToList(); diff --git a/Tests/Dfe.Academies.Testing.Common/Customizations/Models/AcademyGovernanceCustomization.cs b/Tests/Dfe.Academies.Testing.Common/Customizations/Models/AcademyGovernanceCustomization.cs index 1e7c1229b..65b9e37b1 100644 --- a/Tests/Dfe.Academies.Testing.Common/Customizations/Models/AcademyGovernanceCustomization.cs +++ b/Tests/Dfe.Academies.Testing.Common/Customizations/Models/AcademyGovernanceCustomization.cs @@ -5,16 +5,24 @@ namespace Dfe.Academies.Testing.Common.Customizations.Models { public class AcademyGovernanceCustomization : ICustomization { + public string? FirstName { get; set; } = "John"; + public string? LastName { get; set; } = "Doe"; + public string? Email { get; set; } = "john.doe@example.com"; + public string? DisplayName { get; set; } = "John Doe"; + public string? DisplayNameWithTitle { get; set; } = "Mr. John Doe"; + public List Roles { get; set; } = ["MP"]; + public DateTime UpdatedAt { get; set; } = DateTime.Now; + public void Customize(IFixture fixture) { fixture.Customize(composer => composer - .With(x => x.FirstName, "John") - .With(x => x.LastName, "Doe") - .With(x => x.Email, "john.doe@example.com") - .With(x => x.DisplayName, "John Doe") - .With(x => x.DisplayNameWithTitle, "Mr. John Doe") - .With(x => x.Roles, new List { "MP" }) - .With(x => x.UpdatedAt, DateTime.Now)); + .With(x => x.FirstName, FirstName) + .With(x => x.LastName, LastName) + .With(x => x.Email, Email) + .With(x => x.DisplayName, DisplayName) + .With(x => x.DisplayNameWithTitle, DisplayNameWithTitle) + .With(x => x.Roles, Roles) + .With(x => x.UpdatedAt, UpdatedAt)); } } } diff --git a/Tests/Dfe.Academies.Testing.Common/Customizations/Models/MemberOfParliamentCustomization.cs b/Tests/Dfe.Academies.Testing.Common/Customizations/Models/MemberOfParliamentCustomization.cs index 0165fd301..6ac5ceea5 100644 --- a/Tests/Dfe.Academies.Testing.Common/Customizations/Models/MemberOfParliamentCustomization.cs +++ b/Tests/Dfe.Academies.Testing.Common/Customizations/Models/MemberOfParliamentCustomization.cs @@ -5,17 +5,26 @@ namespace Dfe.Academies.Testing.Common.Customizations.Models { public class MemberOfParliamentCustomization : ICustomization { + public string? ConstituencyName { get; set; } + public string? FirstName { get; set; } + public string? LastName { get; set; } + public string? Email { get; set; } + public string? DisplayName { get; set; } + public string? DisplayNameWithTitle { get; set; } + public List? Roles { get; set; } + public DateTime? UpdatedAt { get; set; } + public void Customize(IFixture fixture) { fixture.Customize(composer => composer - .With(x => x.ConstituencyName, "ExampleConstituency") - .With(x => x.FirstName, "John") - .With(x => x.LastName, "Doe") - .With(x => x.Email, "john.doe@example.com") - .With(x => x.DisplayName, "John Doe") - .With(x => x.DisplayNameWithTitle, "Mr. John Doe") - .With(x => x.Roles, new List { "MP" }) - .With(x => x.UpdatedAt, DateTime.Now)); + .With(x => x.ConstituencyName, ConstituencyName ?? fixture.Create()) + .With(x => x.FirstName, FirstName ?? fixture.Create()) + .With(x => x.LastName, LastName ?? fixture.Create()) + .With(x => x.Email, Email ?? fixture.Create()) + .With(x => x.DisplayName, DisplayName ?? fixture.Create()) + .With(x => x.DisplayNameWithTitle, DisplayNameWithTitle ?? fixture.Create()) + .With(x => x.Roles, Roles ?? fixture.Create>()) + .With(x => x.UpdatedAt, UpdatedAt ?? fixture.Create())); } } } diff --git a/Tests/Dfe.Academies.Testing.Common/Customizations/OmitCircularReferenceCustomization.cs b/Tests/Dfe.Academies.Testing.Common/Customizations/OmitCircularReferenceCustomization.cs new file mode 100644 index 000000000..40ff7b986 --- /dev/null +++ b/Tests/Dfe.Academies.Testing.Common/Customizations/OmitCircularReferenceCustomization.cs @@ -0,0 +1,15 @@ +using AutoFixture; + +namespace Dfe.Academies.Testing.Common.Customizations +{ + public class OmitCircularReferenceCustomization : ICustomization + { + public void Customize(IFixture fixture) + { + fixture.Behaviors.OfType().ToList() + .ForEach(b => fixture.Behaviors.Remove(b)); + + fixture.Behaviors.Add(new OmitOnRecursionBehavior()); + } + } +} diff --git a/Tests/Dfe.Academies.Testing.Common/Dfe.Academies.Testing.Common.csproj b/Tests/Dfe.Academies.Testing.Common/Dfe.Academies.Testing.Common.csproj index e434be00e..4a3a8a2d6 100644 --- a/Tests/Dfe.Academies.Testing.Common/Dfe.Academies.Testing.Common.csproj +++ b/Tests/Dfe.Academies.Testing.Common/Dfe.Academies.Testing.Common.csproj @@ -24,6 +24,9 @@ + + +