From 42c288a079c2f3fad208075250e123e0e4087315 Mon Sep 17 00:00:00 2001 From: CLAWLOR Date: Wed, 6 Nov 2024 17:09:58 +0000 Subject: [PATCH 1/6] Modify tests totest the actual factory rather than a mock of the facrory --- .../Factories/LogicalOperatorFactory.cs | 2 + .../Factories/LogicalOperatorFactoryTests.cs | 37 ++++++++++++------- .../TestDoubles/ILogicalOperatorMock.cs | 13 +++++++ 3 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/ILogicalOperatorMock.cs diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs index ac34542..034911e 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs @@ -50,6 +50,7 @@ public LogicalOperatorFactory(Dictionary> logical /// /// The configured instance of the type. /// + // TODO - can we remove this because I thought it was actually going to try to get an operator by type public ILogicalOperator CreateLogicalOperator() where TLogicalOperator : ILogicalOperator => CreateLogicalOperator(typeof(TLogicalOperator)); @@ -62,6 +63,7 @@ public ILogicalOperator CreateLogicalOperator() /// /// The configured instance of the type. /// + // TODO - can we remove this because I thought it was actually going to try to get an operator by type public ILogicalOperator CreateLogicalOperator(Type logicalOperatorType) => CreateLogicalOperator(logicalOperatorName: logicalOperatorType.Name); diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs index 0630a80..f5d1018 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs @@ -1,6 +1,7 @@ using Dfe.Data.Common.Infrastructure.CognitiveSearch.Filtering.LogicalOperators; using Dfe.Data.Common.Infrastructure.CognitiveSearch.Filtering.LogicalOperators.Factories; using Dfe.Data.Common.Infrastructure.CognitiveSearch.Tests.Filtering.FilterExpressions; +using Dfe.Data.Common.Infrastructure.CognitiveSearch.Tests.Filtering.TestDoubles; using FluentAssertions; using Xunit; @@ -12,12 +13,11 @@ public sealed class LogicalOperatorFactoryTests public void CreateLogicalOperator_WithValidOperatorName_ReturnsExpectedConfiguredOperatorFromfactory() { // arrange. - ILogicalOperatorFactory logicalOperatorFactory = - LogicalOperatorFactoryTestDouble.MockLogicalOperatorFactory(); + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); // act. ILogicalOperator logicalOperator = - logicalOperatorFactory.CreateLogicalOperator("AndLogicalOperator"); + logicalOperatorFactory.CreateLogicalOperator("name1"); // assert. logicalOperator.Should().NotBeNull(); @@ -28,8 +28,7 @@ public void CreateLogicalOperator_WithValidOperatorName_ReturnsExpectedConfigure public void CreateLogicalOperator_WithValidOperatorType_ReturnsExpectedConfiguredOperatorFromfactory() { // arrange. - ILogicalOperatorFactory logicalOperatorFactory = - LogicalOperatorFactoryTestDouble.MockLogicalOperatorFactory(); + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); // act. ILogicalOperator logicalOperator = @@ -44,8 +43,7 @@ public void CreateLogicalOperator_WithValidOperatorType_ReturnsExpectedConfigure public void CreateLogicalOperator_WithValidOperatorGenricTemplateType_ReturnsExpectedConfiguredOperatorFromfactory() { // arrange. - ILogicalOperatorFactory logicalOperatorFactory = - LogicalOperatorFactoryTestDouble.MockLogicalOperatorFactoryFor(); + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); // act. ILogicalOperator logicalOperator = @@ -56,12 +54,26 @@ public void CreateLogicalOperator_WithValidOperatorGenricTemplateType_ReturnsExp logicalOperator.Should().BeOfType(); } + [Fact] + public void CreateFilter_WithUnknownFilterName_ThrowsExpectedException() + { + // arrange. + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); + + // act. + Action failedCreateOperatorAction = () => + logicalOperatorFactory.CreateLogicalOperator(logicalOperatorName: "something else"); + + //assert + ArgumentException exception = + Assert.Throws(failedCreateOperatorAction); + } + [Fact] public void CreateFilter_WithNullFilterName_ThrowsExpectedException() { // arrange. - ILogicalOperatorFactory logicalOperatorFactory = - LogicalOperatorFactoryTestDouble.MockLogicalOperatorFactory(); + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); // act. Action failedCreateOperatorAction = () => @@ -78,9 +90,7 @@ public void CreateFilter_WithNullFilterName_ThrowsExpectedException() public void CreateFilter_WithEmptyFilterName_ThrowsExpectedException() { // arrange. - ILogicalOperatorFactory logicalOperatorFactory = - LogicalOperatorFactoryTestDouble.MockLogicalOperatorFactory(); - + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); // act. Action failedCreateOperatorAction = () => logicalOperatorFactory.CreateLogicalOperator(logicalOperatorName: ""); @@ -96,8 +106,7 @@ public void CreateFilter_WithEmptyFilterName_ThrowsExpectedException() public void CreateFilter_WithWhitespaceFilterName_ThrowsExpectedException() { // arrange. - ILogicalOperatorFactory logicalOperatorFactory = - LogicalOperatorFactoryTestDouble.MockLogicalOperatorFactory(); + ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); // act. Action failedCreateOperatorAction = () => diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/ILogicalOperatorMock.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/ILogicalOperatorMock.cs new file mode 100644 index 0000000..a042090 --- /dev/null +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/ILogicalOperatorMock.cs @@ -0,0 +1,13 @@ +using Dfe.Data.Common.Infrastructure.CognitiveSearch.Filtering.LogicalOperators; + +namespace Dfe.Data.Common.Infrastructure.CognitiveSearch.Tests.Filtering.TestDoubles; + +public static class ILogicalOperators +{ + public static Dictionary> Create() + { + return new() { + { "name1", () => new AndLogicalOperator()} + }; + } +} From 99bbe9b12e64f48ee493368c431b8db9cadedca6 Mon Sep 17 00:00:00 2001 From: Spencer O'HEGARTY Date: Wed, 6 Nov 2024 17:25:37 +0000 Subject: [PATCH 2/6] Removed unused type code --- .../Factories/ILogicalOperatorFactory.cs | 22 ------------- .../Factories/LogicalOperatorFactory.cs | 26 ---------------- .../Factories/LogicalOperatorFactoryTests.cs | 31 ------------------- .../LogicalOperatorFactoryTestDouble.cs | 22 ------------- 4 files changed, 101 deletions(-) diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/ILogicalOperatorFactory.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/ILogicalOperatorFactory.cs index f360edc..250bb7b 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/ILogicalOperatorFactory.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/ILogicalOperatorFactory.cs @@ -7,17 +7,6 @@ /// public interface ILogicalOperatorFactory { - /// - /// Allows creation of an instance based on the type requested. - /// - /// - /// The concrete implementation type of requested. - /// - /// - /// The configured instance of the type. - /// - ILogicalOperator CreateLogicalOperator(Type logicalOperatorType); - /// /// Allows creation of an instance based on the type name requested. /// @@ -28,15 +17,4 @@ public interface ILogicalOperatorFactory /// The configured instance of the type. /// ILogicalOperator CreateLogicalOperator(string logicalOperatorName); - - /// - /// Allows creation of an instance based on the generic type specified. - /// - /// - /// The concrete type of requested. - /// - /// - /// The configured instance of the type. - /// - ILogicalOperator CreateLogicalOperator() where TLogicalOperator : ILogicalOperator; } diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs index 034911e..57f0afe 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Filtering/LogicalOperators/Factories/LogicalOperatorFactory.cs @@ -41,32 +41,6 @@ public LogicalOperatorFactory(Dictionary> logical _logicalOperatorFactory = logicalOperatorFactory; } - /// - /// Allows creation of an instance based on the generic type specified. - /// - /// - /// The concrete type of requested. - /// - /// - /// The configured instance of the type. - /// - // TODO - can we remove this because I thought it was actually going to try to get an operator by type - public ILogicalOperator CreateLogicalOperator() - where TLogicalOperator : ILogicalOperator => CreateLogicalOperator(typeof(TLogicalOperator)); - - /// - /// Allows creation of an instance based on the type requested. - /// - /// - /// The concrete implementation type of requested. - /// - /// - /// The configured instance of the type. - /// - // TODO - can we remove this because I thought it was actually going to try to get an operator by type - public ILogicalOperator CreateLogicalOperator(Type logicalOperatorType) => CreateLogicalOperator(logicalOperatorName: logicalOperatorType.Name); - - /// /// Allows creation of an instance based on the type name requested. /// diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs index f5d1018..c0ff748 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/LogicalOperators/Factories/LogicalOperatorFactoryTests.cs @@ -1,6 +1,5 @@ using Dfe.Data.Common.Infrastructure.CognitiveSearch.Filtering.LogicalOperators; using Dfe.Data.Common.Infrastructure.CognitiveSearch.Filtering.LogicalOperators.Factories; -using Dfe.Data.Common.Infrastructure.CognitiveSearch.Tests.Filtering.FilterExpressions; using Dfe.Data.Common.Infrastructure.CognitiveSearch.Tests.Filtering.TestDoubles; using FluentAssertions; using Xunit; @@ -24,36 +23,6 @@ public void CreateLogicalOperator_WithValidOperatorName_ReturnsExpectedConfigure logicalOperator.Should().BeOfType(); } - [Fact] - public void CreateLogicalOperator_WithValidOperatorType_ReturnsExpectedConfiguredOperatorFromfactory() - { - // arrange. - ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); - - // act. - ILogicalOperator logicalOperator = - logicalOperatorFactory.CreateLogicalOperator(typeof(AndLogicalOperator)); - - // assert. - logicalOperator.Should().NotBeNull(); - logicalOperator.Should().BeOfType(); - } - - [Fact] - public void CreateLogicalOperator_WithValidOperatorGenricTemplateType_ReturnsExpectedConfiguredOperatorFromfactory() - { - // arrange. - ILogicalOperatorFactory logicalOperatorFactory = new LogicalOperatorFactory(ILogicalOperators.Create()); - - // act. - ILogicalOperator logicalOperator = - logicalOperatorFactory.CreateLogicalOperator(); - - // assert. - logicalOperator.Should().NotBeNull(); - logicalOperator.Should().BeOfType(); - } - [Fact] public void CreateFilter_WithUnknownFilterName_ThrowsExpectedException() { diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/LogicalOperatorFactoryTestDouble.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/LogicalOperatorFactoryTestDouble.cs index 072ef80..9adfdad 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/LogicalOperatorFactoryTestDouble.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/Filtering/TestDoubles/LogicalOperatorFactoryTestDouble.cs @@ -21,28 +21,6 @@ public static ILogicalOperatorFactory MockLogicalOperatorFactoryFor() .Returns((string type) => GetLogicalOperator(type)); - logicalOperatorFactoryMock - .Setup(logicalOperatorFactory => - logicalOperatorFactory - .CreateLogicalOperator(It.IsAny())) - .Returns((Type type) => - GetLogicalOperator(type.Name)); - - return logicalOperatorFactoryMock.Object; - } - - public static ILogicalOperatorFactory MockLogicalOperatorFactoryFor() - { - var logicalOperatorFactoryMock = LogicalOperatorFactoryMock(); - - logicalOperatorFactoryMock - .Setup(logicalOperatorFactory => - logicalOperatorFactory - .CreateLogicalOperator()) - .Returns(() => - GetLogicalOperator( - typeof(TFilterType).Name)); - return logicalOperatorFactoryMock.Object; } From 65783e91aad1278f5e4a6da0a1a3614269a02bf4 Mon Sep 17 00:00:00 2001 From: Spencer O'HEGARTY Date: Thu, 7 Nov 2024 07:40:04 +0000 Subject: [PATCH 3/6] Fix to broken composition root test --- .../Tests/CompositionRootTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs index ff9ccbe..2f6c170 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs @@ -78,7 +78,7 @@ public async Task AddAzureGeoLocationSearchServices_RegistersAllDependencies() IGeoLocationClientProvider mockGeoLocationClientProvider = GeoLocationClientProviderTestDouble - .CreateWithHttpStatusAndResponse(HttpStatusCode.OK, "{}"); + .CreateWithHttpStatusAndResponse(HttpStatusCode.OK, "{ }"); IServiceProvider? serviceProvider = _compositionRootServiceProvider? From b2922a216b0d8979b5ee17b4504915e9fa633906 Mon Sep 17 00:00:00 2001 From: Spencer O'HEGARTY Date: Thu, 7 Nov 2024 07:52:49 +0000 Subject: [PATCH 4/6] removed async from tests where not using await to clear warnings --- .../Tests/CompositionRootTests.cs | 2 +- .../Tests/ConfigurationStartupValidationTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs index 2f6c170..c005d53 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs @@ -98,7 +98,7 @@ public async Task AddAzureGeoLocationSearchServices_RegistersAllDependencies() } [Fact] - public async Task AddAzureSearchFilterServices_RegistersAllDependencies() + public void AddAzureSearchFilterServices_RegistersAllDependencies() { Dictionary config = new() { { "FilterKeyToFilterExpressionMapOptions:FilterChainingLogicalOperator", "AndLogicalOperator" }, diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/ConfigurationStartupValidationTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/ConfigurationStartupValidationTests.cs index 4a0d5f2..0d4e52f 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/ConfigurationStartupValidationTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/ConfigurationStartupValidationTests.cs @@ -147,7 +147,7 @@ public void AddAzureGeoLocationSearchServices_MissingGeoLocationOptions_MapsSubs } [Fact] - public async Task AddAzureSearchFilterServices_MissingFilterKeyToFilterExpressionMapOptions_FilterChainingLogicalOperator_ThrowOptionsValidationException() + public void AddAzureSearchFilterServices_MissingFilterKeyToFilterExpressionMapOptions_FilterChainingLogicalOperator_ThrowOptionsValidationException() { Dictionary config = new() { { "FilterKeyToFilterExpressionMapOptions:SearchFilterToExpressionMap:test-key:FilterExpressionKey", "SearchInFilterExpression" }, @@ -173,7 +173,7 @@ public async Task AddAzureSearchFilterServices_MissingFilterKeyToFilterExpressio } [Fact] - public async Task AddAzureSearchFilterServices_MissingFilterKeyToFilterExpressionMapOptions_SearchFilterToExpressionMap_ThrowOptionsValidationException() + public void AddAzureSearchFilterServices_MissingFilterKeyToFilterExpressionMapOptions_SearchFilterToExpressionMap_ThrowOptionsValidationException() { Dictionary config = new() { { "FilterKeyToFilterExpressionMapOptions:FilterChainingLogicalOperator", "AndLogicalOperator" } From 77765e156d27802560216cc91665118740218027 Mon Sep 17 00:00:00 2001 From: Spencer O'HEGARTY Date: Thu, 7 Nov 2024 07:57:42 +0000 Subject: [PATCH 5/6] Fixed warning in composition root tests --- .../Tests/CompositionRootTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs index c005d53..74e81bc 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs @@ -58,7 +58,7 @@ await searchByKeywordService searchKeyword: "Test", searchIndex: "Index", searchOptions: new Azure.Search.Documents.SearchOptions() - ); + )!; response.Should().NotBeNull(); } From cc2e2db77d0b5b5e1c4fafd5ee45f37e99a8e7cd Mon Sep 17 00:00:00 2001 From: CLAWLOR Date: Mon, 18 Nov 2024 08:22:40 +0000 Subject: [PATCH 6/6] remove build waring in test --- .../Tests/CompositionRootTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs index 74e81bc..d230be2 100644 --- a/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs +++ b/Dfe.Data.Common.Infrastructure.CognitiveSearch/Tests/CompositionRootTests.cs @@ -53,7 +53,7 @@ public async Task AddAzureSearchServices_RegistersAllDependencies() // act var response = - await searchByKeywordService + await searchByKeywordService! .SearchAsync( searchKeyword: "Test", searchIndex: "Index",