PR Review Changes

PR Reviews from Juan pt. 2 updated schema filter tests Changes per PR reviews Pt. 3
Green-Software-Foundation · Feb 21, 2023 · fddc05f · fddc05f
1 parent 8e9695b
commit fddc05f
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 109 deletions.
diff --git a/...onAware.DataSources.ElectricityMaps/test/Configuration/ServiceCollectionExtensionTests.cs b/...onAware.DataSources.ElectricityMaps/test/Configuration/ServiceCollectionExtensionTests.cs
@@ -9,15 +9,18 @@ namespace CarbonAware.DataSources.ElectricityMaps.Tests;
 [TestFixture]
 public class ServiceCollectionExtensionTests
 {
-    private readonly string ForecastDataSourceKey = $"DataSources:ForecastDataSource";
-    private readonly string EmissionsDataSourceKey = $"DataSources:EmissionsDataSource";
-    private readonly string ForecastDataSourceValue = $"ElectricityMapsTest";
-    private readonly string EmissionsDataSourceValue = $"ElectricityMapsTest";
-    private readonly string HeaderKey = $"DataSources:Configurations:ElectricityMapsTest:APITokenHeader";
-    private readonly string AuthHeader = "auth-token";
-    private readonly string TokenKey = $"DataSources:Configurations:ElectricityMapsTest:APIToken";
-    private readonly string DefaultTokenValue = "myDefaultToken123";
-    private readonly string UseProxyKey = $"DataSources:Configurations:ElectricityMapsTest:Proxy:UseProxy";
+    const string ForecastDataSourceKey = $"DataSources:ForecastDataSource";
+    const string EmissionsDataSourceKey = $"DataSources:EmissionsDataSource";
+    const string ForecastDataSourceValue = $"ElectricityMapsTest";
+    const string EmissionsDataSourceValue = $"ElectricityMapsTest";
+    const string HeaderKey = $"DataSources:Configurations:ElectricityMapsTest:APITokenHeader";
+    const string AuthHeader = "auth-token";
+    const string TokenKey = $"DataSources:Configurations:ElectricityMapsTest:APIToken";
+    const string DefaultTokenValue = "myDefaultToken123";
+    const string UseProxyKey = $"DataSources:Configurations:ElectricityMapsTest:Proxy:UseProxy";
+    const string ProxyUrl = $"DataSources:Configurations:ElectricityMapsTest:Proxy:Url";
+    const string ProxyUsername = $"DataSources:Configurations:ElectricityMapsTest:Proxy:Username";
+    const string ProxyPassword = $"DataSources:Configurations:ElectricityMapsTest:Proxy:Password";
 
     [Test]
     public void ClientProxyTest_With_Missing_ProxyURL_ThrowsException()
@@ -41,58 +44,38 @@ public void ClientProxyTest_With_Missing_ProxyURL_ThrowsException()
         Assert.Throws<ConfigurationException>(() => serviceCollection.AddElectricityMapsEmissionsDataSource(configuration.DataSources()));
     }
 
-    [Test]
-    public void ClientProxyTest_With_No_ProxyURL_AddsServices()
+    [TestCase(true, TestName = "ClientProxyTest, successful: denotes adding ElectricityMaps data sources using proxy.")]
+    [TestCase(false, TestName = "ClientProxyTest, successful: denotes adding ElectricityMaps data sources without using proxy.")]
+    public void ClientProxy_ConfigurationTest(bool withProxyUrl)
     {
         // Arrange
         var settings = new Dictionary<string, string> {
             { ForecastDataSourceKey, ForecastDataSourceValue },
             { EmissionsDataSourceKey, EmissionsDataSourceValue },
             { HeaderKey, AuthHeader },
             { TokenKey, DefaultTokenValue },
-            { UseProxyKey, "false" },
+            { UseProxyKey, withProxyUrl.ToString() }
         };
-        var configuration = new ConfigurationBuilder()
-            .AddInMemoryCollection(settings)
-            .AddEnvironmentVariables()
-            .Build();
-        var serviceCollection = new ServiceCollection();
-
-        // Act
-        var forecastService = serviceCollection.AddElectricityMapsForecastDataSource(configuration.DataSources());
-        var emissionsService = serviceCollection.AddElectricityMapsEmissionsDataSource(configuration.DataSources());
 
-        // Assert
-        Assert.NotNull(forecastService);
-        Assert.NotNull(emissionsService);
-    }
+        if (withProxyUrl)
+        {
+            settings.Add(ProxyUrl, "http://fakeProxy");
+            settings.Add(ProxyUsername, "proxyUsername");
+            settings.Add(ProxyPassword, "proxyPassword");
+        }
 
-    [Test]
-    public void ClientProxyTest_With_ProxyURL_AddsServices()
-    {
-        // Arrange
-        var settings = new Dictionary<string, string> {
-            { ForecastDataSourceKey, ForecastDataSourceValue },
-            { EmissionsDataSourceKey, EmissionsDataSourceValue },
-            { HeaderKey, AuthHeader },
-            { TokenKey, DefaultTokenValue },
-            { UseProxyKey, "true" },
-            { "DataSources:Configurations:ElectricityMapsTest:Proxy:url", "http://10.10.10.1"},
-            { "DataSources:Configurations:ElectricityMapsTest:Proxy:username", "proxyUsername"},
-            { "DataSources:Configurations:ElectricityMapsTest:Proxy:password", "proxyPassword"}
-        };
         var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(settings)
             .AddEnvironmentVariables()
             .Build();
         var serviceCollection = new ServiceCollection();
 
         // Act
-        var forecastService = serviceCollection.AddElectricityMapsForecastDataSource(configuration.DataSources());
-        var emissionsService = serviceCollection.AddElectricityMapsEmissionsDataSource(configuration.DataSources());
+        var forecastDataSource = serviceCollection.AddElectricityMapsForecastDataSource(configuration.DataSources());
+        var emissionsDataSource = serviceCollection.AddElectricityMapsEmissionsDataSource(configuration.DataSources());
 
         // Assert
-        Assert.NotNull(forecastService);
-        Assert.NotNull(emissionsService);
+        Assert.That(forecastDataSource, Is.Not.Null);
+        Assert.That(emissionsDataSource, Is.Not.Null);
     }
 }
diff --git a/src/CarbonAware.DataSources/CarbonAware.DataSources.Json/test/JsonDataSourceTests.cs b/src/CarbonAware.DataSources/CarbonAware.DataSources.Json/test/JsonDataSourceTests.cs
@@ -101,8 +101,8 @@ public async Task GetCarbonIntensityAsync_ReturnsEmptyEmissionData()
         var end = DateTimeOffset.Parse("2022-09-07T13:45:11+00:00");
         var dataSource = mockDataSource.Object;
         var result = await dataSource.GetCarbonIntensityAsync(locations, start, end);
-        Assert.AreEqual(0, result.Count());
-        Assert.IsTrue(!result.Any());
+        Assert.That(result.Count(), Is.EqualTo(0));
+        Assert.That(!result.Any(), Is.True);
     }
 
     private Mock<JsonDataSource> SetupMockDataSource() {

diff --git a/...es/CarbonAware.DataSources.WattTime/test/Configuration/ServiceCollectionExtensionTests.cs b/...es/CarbonAware.DataSources.WattTime/test/Configuration/ServiceCollectionExtensionTests.cs
@@ -22,6 +22,8 @@ public class ServiceCollectionExtensionTests
     private readonly string PasswordKey = $"DataSources:Configurations:WattTimeTest:Password";
     private readonly string Password = "12345";
     private readonly string ProxyUrl = $"DataSources:Configurations:WattTimeTest:Proxy:Url";
+    private readonly string ProxyUsername = $"DataSources:Configurations:WattTimeTest:Proxy:Username";
+    private readonly string ProxyPassword = $"DataSources:Configurations:WattTimeTest:Proxy:Password";
     private readonly string UseProxyKey = $"DataSources:Configurations:WattTimeTest:Proxy:UseProxy";
 
     [Test]
@@ -73,30 +75,38 @@ public void ClientProxyTest_With_Missing_ProxyURL_ThrowsException()
         Assert.Throws<ConfigurationException>(() => serviceCollection.AddWattTimeEmissionsDataSource(configuration.DataSources()));
     }
 
-    [Test]
-    public void ClientProxyTest_With_Missing_ProxyURL_AddsDataSource()
+    [TestCase(true, TestName = "ClientProxyTest, successful: denotes adding WattTime data sources using proxy.")]
+    [TestCase(false, TestName = "ClientProxyTest, successful: denotes adding WattTime data sources without using proxy.")]
+    public void ClientProxyTest_AddsDataSource(bool withProxyUrl)
     {
         // Arrange
         var settings = new Dictionary<string, string> {
             { ForecastDataSourceKey, ForecastDataSourceValue },
             { EmissionsDataSourceKey, EmissionsDataSourceValue },
             { UsernameKey, Username },
             { PasswordKey, Password },
-            { UseProxyKey, "false" },
+            { UseProxyKey, withProxyUrl.ToString() }
         };
 
+        if (withProxyUrl)
+        {
+            settings.Add(ProxyUrl, "http://10.10.10.1");
+            settings.Add(ProxyUsername, "proxyUsername");
+            settings.Add(ProxyPassword, "proxyPassword");
+        }
+
         var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(settings)
             .AddEnvironmentVariables()
             .Build();
         var serviceCollection = new ServiceCollection();
 
         // Act
-        var forecastService = serviceCollection.AddWattTimeEmissionsDataSource(configuration.DataSources());
-        var emissionsService = serviceCollection.AddWattTimeForecastDataSource(configuration.DataSources());
+        var forecastDataSource = serviceCollection.AddWattTimeEmissionsDataSource(configuration.DataSources());
+        var emissionsDataSource = serviceCollection.AddWattTimeForecastDataSource(configuration.DataSources());
 
         // Assert
-        Assert.NotNull(forecastService);
-        Assert.NotNull(emissionsService);
+        Assert.That(forecastDataSource, Is.Not.Null);
+        Assert.That(emissionsDataSource, Is.Not.Null);
     }
 }
diff --git a/src/CarbonAware.LocationSources/test/LocationSourceTest.cs b/src/CarbonAware.LocationSources/test/LocationSourceTest.cs
@@ -245,9 +245,9 @@ public async Task GetGeopositionLocationsAsync_ReturnsListOfLocations()
         var logger = Mock.Of<ILogger<LocationSource>>();
         var locationSource = new LocationSource(logger, options.Object);
 
-        IDictionary<string, Location> allLocations = await locationSource.GetGeopositionLocationsAsync();
+        var allLocations = await locationSource.GetGeopositionLocationsAsync();
 
-        Assert.AreEqual(allLocations.Count, 3);
+        Assert.That(allLocations.Count, Is.EqualTo(3));
         AssertLocationsEqual(Constants.LocationEastUs, allLocations["prefix-test-eastus"]);
         AssertLocationsEqual(Constants.LocationWestUs, allLocations["prefix-test-westus"]);
     }

diff --git a/src/CarbonAware.WebApi/test/unitTests/Configuration/ServiceCollectionExtensionsTests.cs b/src/CarbonAware.WebApi/test/unitTests/Configuration/ServiceCollectionExtensionsTests.cs
@@ -13,75 +13,73 @@ public class ServiceCollectionExtensionsTests
     public void AddMonitoringAndTelemetry_AddsServices_WithConnectionString()
     {
         // Arrange
-        IServiceCollection services = new ServiceCollection();
+        var services = new ServiceCollection();
 
         var inMemorySettings = new Dictionary<string, string>
         {
             { "CarbonAwareVars:TelemetryProvider", "ApplicationInsights" },
             { "ApplicationInsights_Connection_String", "AppInsightsConnectionString" }
         };
-        IConfiguration configuration = new ConfigurationBuilder()
+        var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(inMemorySettings)
             .Build();
 
         // Act & Assert
-        Assert.DoesNotThrow(() => ServiceCollectionExtensions.AddMonitoringAndTelemetry(services, configuration));
-        Assert.AreEqual(services.Count(), 43);
+        Assert.DoesNotThrow(() => services.AddMonitoringAndTelemetry(configuration));
     }
 
     [Test]
     public void AddMonitoringAndTelemetry_AddsServices_WithInstrumentationKey()
     {
         // Arrange
-        IServiceCollection services = new ServiceCollection();
+        var services = new ServiceCollection();
 
         var inMemorySettings = new Dictionary<string, string>
         {
             { "CarbonAwareVars:TelemetryProvider", "ApplicationInsights" },
             { "AppInsights_InstrumentationKey", "AppInsightsInstrumentationKey" }
         };
-        IConfiguration configuration = new ConfigurationBuilder()
+        var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(inMemorySettings)
             .Build();
 
         // Act & Assert
-        Assert.DoesNotThrow(() => ServiceCollectionExtensions.AddMonitoringAndTelemetry(services, configuration));
-        Assert.AreEqual(services.Count(), 43);
+        Assert.DoesNotThrow(() => services.AddMonitoringAndTelemetry(configuration));
     }
 
     [Test]
     public void AddMonitoringAndTelemetry_DoesNotAddServices_WithoutConfiguration()
     {
         // Arrange
-        IServiceCollection services = new ServiceCollection();
+        var services = new ServiceCollection();
 
         var inMemorySettings = new Dictionary<string, string>
         {
             { "CarbonAwareVars:TelemetryProvider", "ApplicationInsights" }
         };
-        IConfiguration configuration = new ConfigurationBuilder()
+        var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(inMemorySettings)
             .Build();
 
         // Act & Assert
-        Assert.DoesNotThrow(() => ServiceCollectionExtensions.AddMonitoringAndTelemetry(services, configuration));
-        Assert.AreEqual(services.Count(), 0);
+        Assert.DoesNotThrow(() => services.AddMonitoringAndTelemetry(configuration));
+        Assert.That(services.Count, Is.EqualTo(0));
     }
 
     [Test]
     public void AddMonitoringAndTelemetry_DoesNotAddServices_WithoutTelemetryProvider()
     {
         // Arrange
-        IServiceCollection services = new ServiceCollection();
+        var services = new ServiceCollection();
 
         var inMemorySettings = new Dictionary<string, string>{};
-        IConfiguration configuration = new ConfigurationBuilder()
+        var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(inMemorySettings)
             .Build();
 
         // Act & Assert
-        Assert.DoesNotThrow(() => ServiceCollectionExtensions.AddMonitoringAndTelemetry(services, configuration));
-        Assert.AreEqual(services.Count(), 0);
+        Assert.DoesNotThrow(() => services.AddMonitoringAndTelemetry(configuration));
+        Assert.That(services.Count, Is.EqualTo(0));
     }    
 
     [Test]
@@ -93,14 +91,14 @@ public void CreateConsoleLogger_ReturnsILogger()
             { "Logging:LogLevel:Default", "Information" },
             { "Logging:LogLevel:Microsoft.AspNetCore", "Warning" }
         };
-        IConfiguration configuration = new ConfigurationBuilder()
+        var configuration = new ConfigurationBuilder()
             .AddInMemoryCollection(inMemorySettings)
             .Build();
 
         // Act
         Microsoft.Extensions.Logging.ILogger logger = ServiceCollectionExtensions.CreateConsoleLogger(configuration);
 
         // Assert
-        Assert.NotNull(logger);
+        Assert.That(logger, Is.Not.Null);
     }
 }
diff --git a/src/CarbonAware.WebApi/test/unitTests/Controllers/CarbonAwareControllerTests.cs b/src/CarbonAware.WebApi/test/unitTests/Controllers/CarbonAwareControllerTests.cs
@@ -288,5 +288,11 @@ public async Task BatchForecastDataAsync_SuccessfulCallReturnsOk()
 
         //Assert
         TestHelpers.AssertStatusCode(result, HttpStatusCode.OK);
+        handler.Verify(a => a.GetForecastByDateAsync(
+            It.IsAny<string>(),
+            It.IsAny<DateTimeOffset>(),
+            It.IsAny<DateTimeOffset>(),
+            It.IsAny<DateTimeOffset>(),
+            It.IsAny<int>()), Times.Once);
     }
 }
diff --git a/...arbonAware.WebApi/test/unitTests/Filters/CarbonAwareParametersBaseDtoSchemaFilterTests.cs b/...arbonAware.WebApi/test/unitTests/Filters/CarbonAwareParametersBaseDtoSchemaFilterTests.cs
@@ -2,8 +2,8 @@
 using Microsoft.OpenApi.Models;
 using NUnit.Framework;
 using Swashbuckle.AspNetCore.SwaggerGen;
-using GSF.CarbonAware.Handlers.CarbonAware;
 using System.Text.Json;
+using CarbonAware.WebApi.Models;
 
 namespace CarbonAware.WepApi.UnitTests;
 
@@ -12,11 +12,8 @@ namespace CarbonAware.WepApi.UnitTests;
 public class CarbonAwareParametersBaseDtoSchemaFilterTests
 {
 
-// Not relevant for tests which populate this field via the [SetUp] attribute.
+    // Not relevant for tests which populate this field via the [SetUp] attribute.
 #pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.
-
-    private OpenApiSchema _schema;
-    private Type _type;
     private SchemaRepository _schemaRepository;
     private ISchemaGenerator _schemaGenerator;
 
@@ -25,25 +22,59 @@ public class CarbonAwareParametersBaseDtoSchemaFilterTests
     [SetUp]
     public void Setup()
     {
-        this._schema = new OpenApiSchema();
-        this._type = typeof(CarbonAwareParametersBaseDTO);
         this._schemaRepository = new SchemaRepository("document-name");
         ISerializerDataContractResolver dataContractResolver = new JsonSerializerDataContractResolver(new JsonSerializerOptions());
         this._schemaGenerator = new SchemaGenerator(new SchemaGeneratorOptions(), dataContractResolver);
     }
 
-    [Test]
-    public void Apply_ReturnsSuccessfully()
+    [TestCase(typeof(CarbonIntensityBatchParametersDTO), new string[] { "MultipleLocations", "Duration", "Requested" }, TestName = "CarbonIntensityBatchParametersDTO, Removing nonOverriddenInheritedProperties")]
+    [TestCase(typeof(CarbonIntensityParametersDTO), new string[] { "MultipleLocations", "Duration", "Requested" }, TestName = "CarbonIntensityParametersDTO, Removing nonOverriddenInheritedProperties")]
+    [TestCase(typeof(EmissionsDataForLocationsParametersDTO), new string[] { "SingleLocation", "Duration", "Requested" }, TestName = "EmissionsDataForLocationsParametersDTO, Removing nonOverriddenInheritedProperties")]
+    [TestCase(typeof(EmissionsForecastBatchParametersDTO), new string[] { "MultipleLocations" }, TestName = "EmissionsForecastBatchParametersDTO, Removing nonOverriddenInheritedProperties")]
+    [TestCase(typeof(EmissionsForecastCurrentParametersDTO), new string[] { "SingleLocation", "Requested" }, TestName = "EmissionsForecastCurrentParametersDTO, Removing nonOverriddenInheritedProperties")]
+    [TestCase(typeof(EmissionsDataDTO), new string[] {}, TestName = "EmissionsDataDTO, Does not remove any properties from non-child class")]
+    public void Apply_RemovesCorrectProperties_FromSchema(Type type, string[] nonOverriddenProperties)
     {
         // Arrange
+        string[] carbonAwareParametersBaseDTOProperties = {
+            "SingleLocation",
+            "MultipleLocations",
+            "Start",
+            "End",
+            "Duration",
+            "Requested"
+        };
+
         var schemaFilterContext = new SchemaFilterContext(
-            _type,
+            type,
             _schemaGenerator,
             _schemaRepository);
 
-        CarbonAwareParametersBaseDtoSchemaFilter caBaseDtoSchemaFilter = new();
+        var caBaseDtoSchemaFilter = new CarbonAwareParametersBaseDtoSchemaFilter();
+
+        var openApiProperties = new Dictionary<string, OpenApiSchema>();
+        foreach (var property in carbonAwareParametersBaseDTOProperties)
+        {
+            openApiProperties.Add(property, new OpenApiSchema());
+        }
+
+        var schema = new OpenApiSchema()
+        {
+            Properties = openApiProperties
+        };
 
         // Act & Assert
-        Assert.DoesNotThrow(() => caBaseDtoSchemaFilter.Apply(_schema, schemaFilterContext));
+        Assert.DoesNotThrow(() => caBaseDtoSchemaFilter.Apply(schema, schemaFilterContext));
+        foreach (var property in carbonAwareParametersBaseDTOProperties)
+        {
+            if(nonOverriddenProperties.Contains(property))
+            {
+                Assert.That(schema.Properties.ContainsKey(property), Is.False);
+            }
+            else
+            {
+                Assert.That(schema.Properties.ContainsKey(property), Is.True);
+            }
+        }
     }
 }