Add environment variable override to force disable an experiment (#4284)

NuGet · Oct 13, 2021 · 4e62c16 · 4e62c16
1 parent 01977bd
commit 4e62c16
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 12 deletions.
diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Experimentation/NuGetExperimentationService.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Experimentation/NuGetExperimentationService.cs
@@ -27,12 +27,19 @@ internal NuGetExperimentationService(IEnvironmentVariableReader environmentVaria
             _experimentationService = experimentationService ?? throw new ArgumentNullException(nameof(experimentationService));
         }
 
-        public bool IsExperimentEnabled(ExperimentationConstants experimentation)
+        public bool IsExperimentEnabled(ExperimentationConstants experiment)
         {
-            var isEnvVarEnabled = !string.IsNullOrEmpty(experimentation.FlightEnvironmentVariable)
-                && _environmentVariableReader.GetEnvironmentVariable(experimentation.FlightEnvironmentVariable) == "1";
+            var isExpForcedEnabled = false;
+            var isExpForcedDisabled = false;
+            if (!string.IsNullOrEmpty(experiment.FlightEnvironmentVariable))
+            {
+                string envVarOverride = _environmentVariableReader.GetEnvironmentVariable(experiment.FlightEnvironmentVariable);
 
-            return isEnvVarEnabled || _experimentationService.IsCachedFlightEnabled(experimentation.FlightFlag);
+                isExpForcedDisabled = envVarOverride == "0";
+                isExpForcedEnabled = envVarOverride == "1";
+            }
+
+            return !isExpForcedDisabled && (isExpForcedEnabled || _experimentationService.IsCachedFlightEnabled(experiment.FlightFlag));
         }
     }
 }
diff --git a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/NuGetExperimentationServiceTests.cs b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/NuGetExperimentationServiceTests.cs
@@ -35,7 +35,7 @@ public void IsEnabled_WithoutEnabledFlight_ReturnsFalse()
         }
 
         [Fact]
-        public void IsEnabled_WithEnabledFlight_WithEnvironmentVariable_ReturnsTrue()
+        public void IsEnabled_WithEnabledFlightAndForcedEnabledEnvVar_ReturnsTrue()
         {
             var constant = ExperimentationConstants.PackageManagerBackgroundColor;
             var envVars = new Dictionary<string, string>()
@@ -49,7 +49,7 @@ public void IsEnabled_WithEnabledFlight_WithEnvironmentVariable_ReturnsTrue()
         }
 
         [Theory]
-        [InlineData("0")]
+        [InlineData("2")]
         [InlineData("randomValue")]
         public void IsEnabled_WithEnvVarWithIncorrectValue_WithEnvironmentVariable__ReturnsFalse(string value)
         {
@@ -77,17 +77,19 @@ public void IsEnabled_WithEnabledFlight_WithExperimentalService_ReturnsTrue()
             service.IsExperimentEnabled(ExperimentationConstants.PackageManagerBackgroundColor).Should().BeTrue();
         }
 
-        [Fact]
-        public void IsEnabled_WithEnvVarWithIncorrectValue_WithExperimentalService_ReturnsFalse()
+        [Theory]
+        [InlineData(true, true)]
+        [InlineData(false, false)]
+        public void IsEnabled_WithEnvVarNotSetAndExperimentalService_ReturnsExpectedResult(bool isFlightEnabled, bool expectedResult)
         {
             var constant = ExperimentationConstants.PackageManagerBackgroundColor;
             var flightsEnabled = new Dictionary<string, bool>()
             {
-                { constant.FlightFlag, false },
+                { constant.FlightFlag, isFlightEnabled },
             };
             var service = new NuGetExperimentationService(new TestEnvironmentVariableReader(new Dictionary<string, string>()), new TestVisualStudioExperimentalService(flightsEnabled));
 
-            service.IsExperimentEnabled(ExperimentationConstants.PackageManagerBackgroundColor).Should().BeFalse();
+            service.IsExperimentEnabled(ExperimentationConstants.PackageManagerBackgroundColor).Should().Be(expectedResult);
         }
 
         [Fact]
@@ -111,7 +113,7 @@ public void IsEnabled_WithEnvVarEnabled_WithExperimentalServiceDisabled_ReturnsT
         }
 
         [Fact]
-        public void IsEnabled_WithEnvVarDisabled_WithExperimentalServiceEnabled_ReturnsTrue()
+        public void IsEnabled_WithEnvVarDisabled_WithExperimentalServiceEnabled_ReturnsFalse()
         {
             var constant = ExperimentationConstants.PackageManagerBackgroundColor;
             var flightsEnabled = new Dictionary<string, bool>()
@@ -127,7 +129,7 @@ public void IsEnabled_WithEnvVarDisabled_WithExperimentalServiceEnabled_ReturnsT
 
             var service = new NuGetExperimentationService(envVarWrapper, new TestVisualStudioExperimentalService(flightsEnabled));
 
-            service.IsExperimentEnabled(ExperimentationConstants.PackageManagerBackgroundColor).Should().BeTrue();
+            service.IsExperimentEnabled(ExperimentationConstants.PackageManagerBackgroundColor).Should().BeFalse();
         }
 
         [Fact]
@@ -136,6 +138,31 @@ public void IsEnabled_WithNullEnvironmentVariableForConstant_HandlesGracefully()
             var service = new NuGetExperimentationService(new EnvironmentVariableWrapper(), new TestVisualStudioExperimentalService());
             service.IsExperimentEnabled(new ExperimentationConstants("flag", null)).Should().BeFalse();
         }
+
+        [Fact]
+        public void IsEnabled_MultipleExperimentsOverriddenWithDifferentEnvVars_DoNotConflict()
+        {
+            var forcedOffExperiment = new ExperimentationConstants("TestExp1", "TEST_EXP_1");
+            var forcedOnExperiment = new ExperimentationConstants("TestExp2", "TEST_EXP_2");
+            var noOverrideExperiment = new ExperimentationConstants("TestExp3", "TEST_EXP_3");
+            var flightsEnabled = new Dictionary<string, bool>()
+            {
+                { forcedOffExperiment.FlightFlag, true },
+                { forcedOnExperiment.FlightFlag, true },
+                { noOverrideExperiment.FlightFlag, true },
+            };
+            var envVars = new Dictionary<string, string>()
+            {
+                { forcedOnExperiment.FlightEnvironmentVariable, "1" },
+                { forcedOffExperiment.FlightEnvironmentVariable, "0" },
+            };
+            var envVarWrapper = new TestEnvironmentVariableReader(envVars);
+            var service = new NuGetExperimentationService(envVarWrapper, new TestVisualStudioExperimentalService(flightsEnabled));
+
+            service.IsExperimentEnabled(forcedOffExperiment).Should().BeFalse();
+            service.IsExperimentEnabled(forcedOnExperiment).Should().BeTrue();
+            service.IsExperimentEnabled(noOverrideExperiment).Should().BeTrue();
+        }
     }
 
     public class TestVisualStudioExperimentalService : IExperimentationService