Improving unit tests (#607)

DFE-Digital · Sep 18, 2024 · 62c3e03 · 62c3e03
1 parent 575969a
commit 62c3e03
Show file tree

Hide file tree

Showing 10 changed files with 123 additions and 41 deletions.
diff --git a/...es/Queries/GetMemberOfParliamentByConstituencies/GetMemberOfParliamentByConstituencies.cs b/...es/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
@@ -10,11 +10,9 @@
 
   <ItemGroup>
     <PackageReference Include="AutoFixture" Version="4.18.1" />
-    <PackageReference Include="FluentAssertions" Version="6.12.0" />
+    <PackageReference Include="FluentAssertions" Version="6.12.1" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
-    <PackageReference Include="Moq" Version="4.20.70" />
-    <PackageReference Include="NSubstitute" Version="5.1.0" />
-    <PackageReference Include="xunit" Version="2.8.1" />
+    <PackageReference Include="xunit" Version="2.9.0" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>

diff --git a/....Tests/QueryHandlers/Constituency/GetMemberOfParliamentByConstituencyQueryHandlerTests.cs b/....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<MemberOfParliament>();
+
             var cacheKey = $"MemberOfParliament_{CacheKeyHelper.GenerateHashedCacheKey(query.ConstituencyName)}";
+
             mockConstituencyRepository.GetMemberOfParliamentByConstituencyAsync(query.ConstituencyName, default)
                 .Returns(constituency);
 
-            mockCacheService.GetOrAddAsync(cacheKey, Arg.Any<Func<Task<MemberOfParliament>>>(), Arg.Any<string>())
-                .Returns(expectedMp);
+            mockCacheService.GetOrAddAsync(
+                    cacheKey,
+                    Arg.Any<Func<Task<MemberOfParliament>>>(),
+                    Arg.Any<string>())
+                .Returns(callInfo =>
+                {
+                    var callback = callInfo.ArgAt<Func<Task<MemberOfParliament>>>(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<Func<Task<MemberOfParliament>>>(), nameof(GetMemberOfParliamentByConstituencyQueryHandler));
+            await mockConstituencyRepository.Received(1).GetMemberOfParliamentByConstituencyAsync(query.ConstituencyName, default);
         }
     }
 }
diff --git a/...sts/QueryHandlers/Constituency/GetMemberOfParliamentsByConstituenciesQueryHandlerTests.cs b/...sts/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<Domain.Constituencies.Constituency> constituencies,
-            List<MemberOfParliament> 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<MemberOfParliament>()).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<Func<Task<List<MemberOfParliament>>>>(), Arg.Any<string>())
-                .Returns(expectedMps);
+                .Returns(callInfo =>
+                {
+                    var callback = callInfo.ArgAt<Func<Task<List<MemberOfParliament>>>>(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<Func<Task<List<MemberOfParliament>>>>(), nameof(GetMembersOfParliamentByConstituenciesQueryHandler));
+            mockConstituencyRepository.Received(1).GetMembersOfParliamentByConstituenciesQueryable(query.ConstituencyNames);
         }
     }
 }
diff --git a/...ication.Tests/QueryHandlers/Establishment/GetAllPersonsAssociatedWithAcademyByUrnTests.cs b/...ication.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<AcademyGovernance> expectedGovernances,
-            IQueryable<AcademyGovernanceQueryModel> governanceQueryModels)
+            List<AcademyGovernanceQueryModel> governanceQueryModels,
+            IFixture fixture)
+
         {
             // Arrange
+            var expectedGovernances = governanceQueryModels.Select(governance =>
+                fixture.Customize(new AcademyGovernanceCustomization
+                {
+                    FirstName = governance?.EducationEstablishmentGovernance?.Forename1,
+                    LastName = governance?.EducationEstablishmentGovernance?.Surname,
+                }).Create<AcademyGovernance>()).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<Func<Task<List<AcademyGovernance>?>>>(), Arg.Any<string>())
-                .Returns(expectedGovernances);
+            mockCacheService.GetOrAddAsync(cacheKey, Arg.Any<Func<Task<List<AcademyGovernance>>>>(), Arg.Any<string>())
+                .Returns(callInfo =>
+                {
+                    var callback = callInfo.ArgAt<Func<Task<List<AcademyGovernance>>>>(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
@@ -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<IMapper>(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
@@ -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<string> Roles { get; set; } = ["MP"];
+        public DateTime UpdatedAt { get; set; } = DateTime.Now;
+
         public void Customize(IFixture fixture)
         {
             fixture.Customize<AcademyGovernance>(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<string> { "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
@@ -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<string>? Roles { get; set; }
+        public DateTime? UpdatedAt { get; set; }
+
         public void Customize(IFixture fixture)
         {
             fixture.Customize<MemberOfParliament>(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<string> { "MP" })
-                .With(x => x.UpdatedAt, DateTime.Now));
+                .With(x => x.ConstituencyName, ConstituencyName ?? fixture.Create<string>())
+                .With(x => x.FirstName, FirstName ?? fixture.Create<string>())
+                .With(x => x.LastName, LastName ?? fixture.Create<string>())
+                .With(x => x.Email, Email ?? fixture.Create<string>())
+                .With(x => x.DisplayName, DisplayName ?? fixture.Create<string>())
+                .With(x => x.DisplayNameWithTitle, DisplayNameWithTitle ?? fixture.Create<string>())
+                .With(x => x.Roles, Roles ?? fixture.Create<List<string>>())
+                .With(x => x.UpdatedAt, UpdatedAt ?? fixture.Create<DateTime>()));
         }
     }
 }
diff --git a/Tests/Dfe.Academies.Testing.Common/Customizations/OmitCircularReferenceCustomization.cs 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<ThrowingRecursionBehavior>().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
@@ -24,6 +24,9 @@
     <PackageReference Include="Microsoft.Data.Sqlite.Core" Version="8.0.8" />
     <PackageReference Include="Microsoft.EntityFrameworkCore" Version="8.0.8" />
     <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="8.0.8" />
+    <PackageReference Include="MockQueryable.NSubstitute" Version="7.0.3" />
+    <PackageReference Include="Moq" Version="4.20.72" />
+    <PackageReference Include="NSubstitute" Version="5.1.0" />
   </ItemGroup>
 
   <ItemGroup>