From 4e62c16ff43cc896aed6260dd57fb144c8448f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Pierre=20Bried=C3=A9?= Date: Wed, 13 Oct 2021 10:37:54 -0700 Subject: [PATCH] Add environment variable override to force disable an experiment (#4284) --- .../NuGetExperimentationService.cs | 15 +++++-- .../NuGetExperimentationServiceTests.cs | 43 +++++++++++++++---- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Experimentation/NuGetExperimentationService.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Experimentation/NuGetExperimentationService.cs index acdf0a28d2f..680f6d7c761 100644 --- 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 index fd8f728463f..0391bc77502 100644 --- 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() @@ -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() { - { constant.FlightFlag, false }, + { constant.FlightFlag, isFlightEnabled }, }; var service = new NuGetExperimentationService(new TestEnvironmentVariableReader(new Dictionary()), 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() @@ -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() + { + { forcedOffExperiment.FlightFlag, true }, + { forcedOnExperiment.FlightFlag, true }, + { noOverrideExperiment.FlightFlag, true }, + }; + var envVars = new Dictionary() + { + { 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