From 4bf24594e8b2a722e7a233116473233c8d58eb68 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 27 Oct 2023 10:34:33 +0800 Subject: [PATCH 1/2] use const string to referece feature name --- tests/Tests.FeatureManagement/FeatureFlags.cs | 19 ++++++ .../FeatureManagement.cs | 68 ++++++++++--------- tests/Tests.FeatureManagement/Features.cs | 19 ------ 3 files changed, 54 insertions(+), 52 deletions(-) create mode 100644 tests/Tests.FeatureManagement/FeatureFlags.cs delete mode 100644 tests/Tests.FeatureManagement/Features.cs diff --git a/tests/Tests.FeatureManagement/FeatureFlags.cs b/tests/Tests.FeatureManagement/FeatureFlags.cs new file mode 100644 index 00000000..a3e94056 --- /dev/null +++ b/tests/Tests.FeatureManagement/FeatureFlags.cs @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +namespace Tests.FeatureManagement +{ + static class FeatureFlags + { + public const string TargetingTestFeature = "TargetingTestFeature"; + public const string TargetingTestFeatureWithExclusion = "TargetingTestFeatureWithExclusion"; + public const string OnTestFeature = "OnTestFeature"; + public const string OffTestFeature = "OffTestFeature"; + public const string ConditionalFeature = "ConditionalFeature"; + public const string ConditionalFeature2 = "ConditionalFeature2"; + public const string ContextualFeature = "ContextualFeature"; + public const string AnyFilterFeature = "AnyFilterFeature"; + public const string AllFilterFeature = "AllFilterFeature"; + public const string FeatureUsesFiltersWithDuplicatedAlias = "FeatureUsesFiltersWithDuplicatedAlias"; + } +} diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 1272af0d..48b16b98 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -33,9 +33,9 @@ public async Task ReadsConfiguration() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - Assert.True(await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.OnTestFeature))); + Assert.True(await featureManager.IsEnabledAsync(FeatureFlags.OnTestFeature)); - Assert.False(await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.OffTestFeature))); + Assert.False(await featureManager.IsEnabledAsync(FeatureFlags.OffTestFeature)); IEnumerable featureFilters = serviceProvider.GetRequiredService>(); @@ -51,12 +51,12 @@ public async Task ReadsConfiguration() Assert.Equal("V1", evaluationContext.Parameters["P1"]); - Assert.Equal(Enum.GetName(typeof(Features), Features.ConditionalFeature), evaluationContext.FeatureName); + Assert.Equal(FeatureFlags.ConditionalFeature, evaluationContext.FeatureName); return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature)); + await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); Assert.True(called); } @@ -64,7 +64,8 @@ public async Task ReadsConfiguration() [Fact] public async Task ReadsOnlyFeatureManagementSection() { - MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes("{\"AllowedHosts\": \"*\"}")); + var stream = new MemoryStream(Encoding.UTF8.GetBytes("{\"AllowedHosts\": \"*\"}")); + IConfiguration config = new ConfigurationBuilder().AddJsonStream(stream).Build(); var services = new ServiceCollection(); @@ -90,7 +91,7 @@ public async Task AllowsDuplicatedFilterAlias() { const string duplicatedFilterName = "DuplicatedFilterName"; - string featureName = Enum.GetName(typeof(Features), Features.FeatureUsesFiltersWithDuplicatedAlias); + string featureName = FeatureFlags.FeatureUsesFiltersWithDuplicatedAlias; IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); @@ -202,6 +203,7 @@ public async Task CustomFilterContextualTargetingWithNullSetting() ServiceCollection services = new ServiceCollection(); var targetingContextAccessor = new OnDemandTargetingContextAccessor(); + services.AddSingleton(targetingContextAccessor); services @@ -219,10 +221,10 @@ public async Task CustomFilterContextualTargetingWithNullSetting() [Fact] public async Task TimeWindow() { - string feature1 = "feature1"; - string feature2 = "feature2"; - string feature3 = "feature3"; - string feature4 = "feature4"; + const string feature1 = "feature1"; + const string feature2 = "feature2"; + const string feature3 = "feature3"; + const string feature4 = "feature4"; Environment.SetEnvironmentVariable($"FeatureManagement:{feature1}:EnabledFor:0:Name", "TimeWindow"); Environment.SetEnvironmentVariable($"FeatureManagement:{feature1}:EnabledFor:0:Parameters:End", DateTimeOffset.UtcNow.AddDays(1).ToString("r")); @@ -256,10 +258,10 @@ public async Task TimeWindow() [Fact] public async Task Percentage() { - string feature1 = "feature1"; + const string feature = "feature"; - Environment.SetEnvironmentVariable($"FeatureManagement:{feature1}:EnabledFor:0:Name", "Percentage"); - Environment.SetEnvironmentVariable($"FeatureManagement:{feature1}:EnabledFor:0:Parameters:Value", "50"); + Environment.SetEnvironmentVariable($"FeatureManagement:{feature}:EnabledFor:0:Name", "Percentage"); + Environment.SetEnvironmentVariable($"FeatureManagement:{feature}:EnabledFor:0:Parameters:Value", "50"); IConfiguration config = new ConfigurationBuilder().AddEnvironmentVariables().Build(); @@ -276,7 +278,7 @@ public async Task Percentage() for (int i = 0; i < 10; i++) { - if (await featureManager.IsEnabledAsync(feature1)) + if (await featureManager.IsEnabledAsync(feature)) { enabledCount++; } @@ -306,7 +308,7 @@ public async Task Targeting() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string targetingTestFeature = Enum.GetName(typeof(Features), Features.TargetingTestFeature); + string targetingTestFeature = FeatureFlags.TargetingTestFeature; // // Targeted by user id @@ -372,7 +374,7 @@ public async Task TargetingAccessor() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string beta = Enum.GetName(typeof(Features), Features.TargetingTestFeature); + string beta = FeatureFlags.TargetingTestFeature; // // Targeted by user id @@ -423,11 +425,11 @@ public async Task UsesContext() context.AccountId = "NotEnabledAccount"; - Assert.False(await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ContextualFeature), context)); + Assert.False(await featureManager.IsEnabledAsync(FeatureFlags.ContextualFeature, context)); context.AccountId = "abc"; - Assert.True(await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ContextualFeature), context)); + Assert.True(await featureManager.IsEnabledAsync(FeatureFlags.ContextualFeature, context)); } [Fact] @@ -495,7 +497,7 @@ public async Task ThrowsExceptionForMissingFeatureFilter() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - FeatureManagementException e = await Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature))); + FeatureManagementException e = await Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature)); Assert.Equal(FeatureManagementError.MissingFeatureFilter, e.Error); } @@ -521,7 +523,7 @@ public async Task SwallowsExceptionForMissingFeatureFilter() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - var isEnabled = await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature)); + var isEnabled = await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); Assert.False(isEnabled); } @@ -556,7 +558,7 @@ public async Task CustomFeatureDefinitionProvider() { FeatureDefinition testFeature = new FeatureDefinition { - Name = Enum.GetName(typeof(Features), Features.ConditionalFeature), + Name = FeatureFlags.ConditionalFeature, EnabledFor = new List() { new FeatureFilterConfiguration @@ -594,12 +596,12 @@ public async Task CustomFeatureDefinitionProvider() Assert.Equal("V1", evaluationContext.Parameters["P1"]); - Assert.Equal(Enum.GetName(typeof(Features), Features.ConditionalFeature), evaluationContext.FeatureName); + Assert.Equal(FeatureFlags.ConditionalFeature, evaluationContext.FeatureName); return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature)); + await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); Assert.True(called); } @@ -641,7 +643,7 @@ public async Task ThreadsafeSnapshot() for (int i = 0; i < 1000; i++) { - tasks.Add(featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature))); + tasks.Add(featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature)); } Assert.True(called); @@ -678,7 +680,7 @@ public async Task TargetingExclusion() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string targetingTestFeature = Enum.GetName(typeof(Features), Features.TargetingTestFeatureWithExclusion); + string targetingTestFeature = FeatureFlags.TargetingTestFeatureWithExclusion; // // Targeted by user id @@ -754,7 +756,7 @@ public async Task UsesRequirementType() { IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); - string filterOneId = "1"; + const string filterOneId = "1"; var services = new ServiceCollection(); @@ -767,8 +769,8 @@ public async Task UsesRequirementType() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string anyFilterFeature = Enum.GetName(typeof(Features), Features.AnyFilterFeature); - string allFilterFeature = Enum.GetName(typeof(Features), Features.AllFilterFeature); + string anyFilterFeature = FeatureFlags.AnyFilterFeature; + string allFilterFeature = FeatureFlags.AllFilterFeature; IEnumerable featureFilters = serviceProvider.GetRequiredService>(); @@ -825,7 +827,7 @@ public async Task RequirementTypeAllExceptions() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string allFilterFeature = Enum.GetName(typeof(Features), Features.AllFilterFeature); + string allFilterFeature = FeatureFlags.AllFilterFeature; await Assert.ThrowsAsync(async () => { @@ -852,7 +854,7 @@ public async Task BindsFeatureFlagSettings() { new FeatureDefinition { - Name = Enum.GetName(typeof(Features), Features.ConditionalFeature), + Name = FeatureFlags.ConditionalFeature, EnabledFor = new List() { testFilterConfiguration @@ -893,7 +895,7 @@ public async Task BindsFeatureFlagSettings() return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature)); + await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); Assert.True(binderCalled); @@ -903,7 +905,7 @@ public async Task BindsFeatureFlagSettings() called = false; - await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature)); + await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); Assert.False(binderCalled); @@ -917,7 +919,7 @@ public async Task BindsFeatureFlagSettings() called = false; - await featureManager.IsEnabledAsync(Enum.GetName(typeof(Features), Features.ConditionalFeature)); + await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); Assert.True(binderCalled); diff --git a/tests/Tests.FeatureManagement/Features.cs b/tests/Tests.FeatureManagement/Features.cs deleted file mode 100644 index 79e1105d..00000000 --- a/tests/Tests.FeatureManagement/Features.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -// -namespace Tests.FeatureManagement -{ - enum Features - { - TargetingTestFeature, - TargetingTestFeatureWithExclusion, - OnTestFeature, - OffTestFeature, - ConditionalFeature, - ConditionalFeature2, - ContextualFeature, - AnyFilterFeature, - AllFilterFeature, - FeatureUsesFiltersWithDuplicatedAlias - } -} From b68dbe53a0e6eefafc5b6a58c24da2a7250e3a1b Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 10 Nov 2023 12:29:29 +0800 Subject: [PATCH 2/2] revert to Features --- .../FeatureManagement.cs | 47 ++++++++++--------- .../{FeatureFlags.cs => Features.cs} | 2 +- 2 files changed, 25 insertions(+), 24 deletions(-) rename tests/Tests.FeatureManagement/{FeatureFlags.cs => Features.cs} (96%) diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 48b16b98..73816617 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -33,9 +33,9 @@ public async Task ReadsConfiguration() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - Assert.True(await featureManager.IsEnabledAsync(FeatureFlags.OnTestFeature)); + Assert.True(await featureManager.IsEnabledAsync(Features.OnTestFeature)); - Assert.False(await featureManager.IsEnabledAsync(FeatureFlags.OffTestFeature)); + Assert.False(await featureManager.IsEnabledAsync(Features.OffTestFeature)); IEnumerable featureFilters = serviceProvider.GetRequiredService>(); @@ -51,12 +51,12 @@ public async Task ReadsConfiguration() Assert.Equal("V1", evaluationContext.Parameters["P1"]); - Assert.Equal(FeatureFlags.ConditionalFeature, evaluationContext.FeatureName); + Assert.Equal(Features.ConditionalFeature, evaluationContext.FeatureName); return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); + await featureManager.IsEnabledAsync(Features.ConditionalFeature); Assert.True(called); } @@ -81,6 +81,7 @@ public async Task ReadsOnlyFeatureManagementSection() await foreach (string featureName in featureManager.GetFeatureNamesAsync()) { + // // Fail, as no features should be found Assert.True(false); } @@ -91,7 +92,7 @@ public async Task AllowsDuplicatedFilterAlias() { const string duplicatedFilterName = "DuplicatedFilterName"; - string featureName = FeatureFlags.FeatureUsesFiltersWithDuplicatedAlias; + string featureName = Features.FeatureUsesFiltersWithDuplicatedAlias; IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); @@ -308,7 +309,7 @@ public async Task Targeting() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string targetingTestFeature = FeatureFlags.TargetingTestFeature; + string targetingTestFeature = Features.TargetingTestFeature; // // Targeted by user id @@ -374,7 +375,7 @@ public async Task TargetingAccessor() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string beta = FeatureFlags.TargetingTestFeature; + string beta = Features.TargetingTestFeature; // // Targeted by user id @@ -425,11 +426,11 @@ public async Task UsesContext() context.AccountId = "NotEnabledAccount"; - Assert.False(await featureManager.IsEnabledAsync(FeatureFlags.ContextualFeature, context)); + Assert.False(await featureManager.IsEnabledAsync(Features.ContextualFeature, context)); context.AccountId = "abc"; - Assert.True(await featureManager.IsEnabledAsync(FeatureFlags.ContextualFeature, context)); + Assert.True(await featureManager.IsEnabledAsync(Features.ContextualFeature, context)); } [Fact] @@ -497,7 +498,7 @@ public async Task ThrowsExceptionForMissingFeatureFilter() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - FeatureManagementException e = await Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature)); + FeatureManagementException e = await Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(Features.ConditionalFeature)); Assert.Equal(FeatureManagementError.MissingFeatureFilter, e.Error); } @@ -523,7 +524,7 @@ public async Task SwallowsExceptionForMissingFeatureFilter() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - var isEnabled = await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); + var isEnabled = await featureManager.IsEnabledAsync(Features.ConditionalFeature); Assert.False(isEnabled); } @@ -558,7 +559,7 @@ public async Task CustomFeatureDefinitionProvider() { FeatureDefinition testFeature = new FeatureDefinition { - Name = FeatureFlags.ConditionalFeature, + Name = Features.ConditionalFeature, EnabledFor = new List() { new FeatureFilterConfiguration @@ -596,12 +597,12 @@ public async Task CustomFeatureDefinitionProvider() Assert.Equal("V1", evaluationContext.Parameters["P1"]); - Assert.Equal(FeatureFlags.ConditionalFeature, evaluationContext.FeatureName); + Assert.Equal(Features.ConditionalFeature, evaluationContext.FeatureName); return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); + await featureManager.IsEnabledAsync(Features.ConditionalFeature); Assert.True(called); } @@ -643,7 +644,7 @@ public async Task ThreadsafeSnapshot() for (int i = 0; i < 1000; i++) { - tasks.Add(featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature)); + tasks.Add(featureManager.IsEnabledAsync(Features.ConditionalFeature)); } Assert.True(called); @@ -680,7 +681,7 @@ public async Task TargetingExclusion() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string targetingTestFeature = FeatureFlags.TargetingTestFeatureWithExclusion; + string targetingTestFeature = Features.TargetingTestFeatureWithExclusion; // // Targeted by user id @@ -769,8 +770,8 @@ public async Task UsesRequirementType() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string anyFilterFeature = FeatureFlags.AnyFilterFeature; - string allFilterFeature = FeatureFlags.AllFilterFeature; + string anyFilterFeature = Features.AnyFilterFeature; + string allFilterFeature = Features.AllFilterFeature; IEnumerable featureFilters = serviceProvider.GetRequiredService>(); @@ -827,7 +828,7 @@ public async Task RequirementTypeAllExceptions() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - string allFilterFeature = FeatureFlags.AllFilterFeature; + string allFilterFeature = Features.AllFilterFeature; await Assert.ThrowsAsync(async () => { @@ -854,7 +855,7 @@ public async Task BindsFeatureFlagSettings() { new FeatureDefinition { - Name = FeatureFlags.ConditionalFeature, + Name = Features.ConditionalFeature, EnabledFor = new List() { testFilterConfiguration @@ -895,7 +896,7 @@ public async Task BindsFeatureFlagSettings() return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); + await featureManager.IsEnabledAsync(Features.ConditionalFeature); Assert.True(binderCalled); @@ -905,7 +906,7 @@ public async Task BindsFeatureFlagSettings() called = false; - await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); + await featureManager.IsEnabledAsync(Features.ConditionalFeature); Assert.False(binderCalled); @@ -919,7 +920,7 @@ public async Task BindsFeatureFlagSettings() called = false; - await featureManager.IsEnabledAsync(FeatureFlags.ConditionalFeature); + await featureManager.IsEnabledAsync(Features.ConditionalFeature); Assert.True(binderCalled); diff --git a/tests/Tests.FeatureManagement/FeatureFlags.cs b/tests/Tests.FeatureManagement/Features.cs similarity index 96% rename from tests/Tests.FeatureManagement/FeatureFlags.cs rename to tests/Tests.FeatureManagement/Features.cs index a3e94056..b52ec008 100644 --- a/tests/Tests.FeatureManagement/FeatureFlags.cs +++ b/tests/Tests.FeatureManagement/Features.cs @@ -3,7 +3,7 @@ // namespace Tests.FeatureManagement { - static class FeatureFlags + static class Features { public const string TargetingTestFeature = "TargetingTestFeature"; public const string TargetingTestFeatureWithExclusion = "TargetingTestFeatureWithExclusion";