From e4df96ebe6a36f066845aad7e83f9c88ef70fa9d Mon Sep 17 00:00:00 2001 From: "Durga Vijay Prabhakar Ogirala (from Dev Box)" Date: Wed, 29 May 2024 09:59:54 +0530 Subject: [PATCH 1/3] Changes to remove local featureflag variable --- .../Legacy/LegacyTestRunDataPublisher.cs | 26 ++++++++++------- .../TestResults/TestDataPublisher.cs | 28 ++++++++++--------- .../TestResults/TestRunDataPublisherHelper.cs | 4 +-- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs b/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs index 63f17d87f0..f53d39bea9 100644 --- a/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs +++ b/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs @@ -3,6 +3,7 @@ using Microsoft.TeamFoundation.TestManagement.WebApi; using Microsoft.VisualStudio.Services.Agent.Util; +using Microsoft.VisualStudio.Services.Agent.Worker.CodeCoverage; using Microsoft.VisualStudio.Services.Agent.Worker.TestResults; using Microsoft.VisualStudio.Services.Agent.Worker.TestResults.Utils; using Microsoft.VisualStudio.Services.WebApi; @@ -34,8 +35,8 @@ public class LegacyTestRunDataPublisher : AgentService, ILegacyTestRunDataPublis private const string _testRunSystemCustomFieldName = "TestRunSystem"; private readonly object _sync = new object(); private int _runCounter = 0; - private IFeatureFlagService _featureFlagService; private bool _calculateTestRunSummary; + private bool _isFlakyCheckEnabled; private string _testRunner; private ITestResultsServer _testResultsServer; private TestRunDataPublisherHelper _testRunPublisherHelper; @@ -48,13 +49,11 @@ public void InitializePublisher(IExecutionContext context, string projectName, V _testRunner = testRunner; _resultReader = GetTestResultReader(_testRunner, publishRunLevelAttachments); _testRunPublisher = HostContext.GetService(); - _featureFlagService = HostContext.GetService(); - _featureFlagService.InitializeFeatureService(_executionContext, connection); _testRunPublisher.InitializePublisher(_executionContext, connection, projectName, _resultReader); _testResultsServer = HostContext.GetService(); _testResultsServer.InitializeServer(connection, _executionContext); - _calculateTestRunSummary = _featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); _testRunPublisherHelper = new TestRunDataPublisherHelper(_executionContext, null, _testRunPublisher, _testResultsServer); + LoadFeatureFlagState(); Trace.Leaving(); } @@ -208,9 +207,7 @@ private async Task PublishAllTestResultsToSingleTestRunAsync(List // Check failed results for flaky aware // Fallback to flaky aware if there are any failures. - bool isFlakyCheckEnabled = _featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); - - if (isTestRunOutcomeFailed && isFlakyCheckEnabled) + if (isTestRunOutcomeFailed && _isFlakyCheckEnabled) { IList publishedRuns = new List(); publishedRuns.Add(updatedRun); @@ -313,9 +310,7 @@ private async Task PublishToNewTestRunPerTestResultFileAsync(List // Check failed results for flaky aware // Fallback to flaky aware if there are any failures. - bool isFlakyCheckEnabled = _featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); - - if (isTestRunOutcomeFailed && isFlakyCheckEnabled) + if (isTestRunOutcomeFailed && _isFlakyCheckEnabled) { var runOutcome = _testRunPublisherHelper.CheckRunsForFlaky(publishedRuns, _projectName); if (runOutcome != null && runOutcome.HasValue) @@ -408,5 +403,16 @@ private bool GetTestRunOutcome(TestRunData testRunData, TestRunSummary testRunSu } return anyFailedTests; } + + private void LoadFeatureFlagState() + { + using (var connection = WorkerUtilities.GetVssConnection(_executionContext)) + { + var featureFlagService = _executionContext.GetHostContext().GetService(); + featureFlagService.InitializeFeatureService(_executionContext, connection); + _calculateTestRunSummary = featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); + _isFlakyCheckEnabled = featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); + } + } } } diff --git a/src/Agent.Worker/TestResults/TestDataPublisher.cs b/src/Agent.Worker/TestResults/TestDataPublisher.cs index af99350378..dfc1383c8d 100644 --- a/src/Agent.Worker/TestResults/TestDataPublisher.cs +++ b/src/Agent.Worker/TestResults/TestDataPublisher.cs @@ -38,9 +38,9 @@ public sealed class TestDataPublisher : AgentService, ITestDataPublisher private IParser _parser; private VssConnection _connection; - private IFeatureFlagService _featureFlagService; private string _testRunner; private bool _calculateTestRunSummary; + private bool _isFlakyCheckEnabled; private TestRunDataPublisherHelper _testRunPublisherHelper; private ITestResultsServer _testResultsServer; @@ -56,10 +56,9 @@ public void InitializePublisher(IExecutionContext context, string projectName, V _testResultsServer = HostContext.GetService(); _testResultsServer.InitializeServer(connection, _executionContext); var extensionManager = HostContext.GetService(); - _featureFlagService = HostContext.GetService(); - _featureFlagService.InitializeFeatureService(_executionContext, connection); _parser = (extensionManager.GetExtensions()).FirstOrDefault(x => _testRunner.Equals(x.Name, StringComparison.OrdinalIgnoreCase)); _testRunPublisherHelper = new TestRunDataPublisherHelper(_executionContext, _testRunPublisher, null, _testResultsServer); + LoadFeatureFlagState(); Trace.Leaving(); } @@ -86,8 +85,6 @@ public void InitializePublisher(IExecutionContext context, string projectName, V IList publishedRuns = publishtestRunDataTask.Result; - _calculateTestRunSummary = _featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); - var isTestRunOutcomeFailed = GetTestRunOutcome(_executionContext, testRunData, out TestRunSummary testRunSummary); // Storing testrun summary in environment variable, which will be read by PublishPipelineMetadataTask and publish to evidence store. @@ -98,9 +95,7 @@ public void InitializePublisher(IExecutionContext context, string projectName, V // Check failed results for flaky aware // Fallback to flaky aware if there are any failures. - bool isFlakyCheckEnabled = _featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); - - if (isTestRunOutcomeFailed && isFlakyCheckEnabled) + if (isTestRunOutcomeFailed && _isFlakyCheckEnabled) { var runOutcome = _testRunPublisherHelper.CheckRunsForFlaky(publishedRuns, _projectName); if (runOutcome != null && runOutcome.HasValue) @@ -188,8 +183,6 @@ public void InitializePublisher(IExecutionContext context, string projectName, V IList publishedRuns = publishtestRunDataTask.Result; - _calculateTestRunSummary = _featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); - var isTestRunOutcomeFailed = GetTestRunOutcome(_executionContext, testRunData, out TestRunSummary testRunSummary); // Storing testrun summary in environment variable, which will be read by PublishPipelineMetadataTask and publish to evidence store. @@ -200,9 +193,7 @@ public void InitializePublisher(IExecutionContext context, string projectName, V // Check failed results for flaky aware // Fallback to flaky aware if there are any failures. - bool isFlakyCheckEnabled = _featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); - - if (isTestRunOutcomeFailed && isFlakyCheckEnabled) + if (isTestRunOutcomeFailed && _isFlakyCheckEnabled) { var runOutcome = _testRunPublisherHelper.CheckRunsForFlaky(publishedRuns, _projectName); if (runOutcome != null && runOutcome.HasValue) @@ -327,5 +318,16 @@ private async Task GetProjectId(string projectName) return proj.Id; } + + private void LoadFeatureFlagState() + { + using (var connection = WorkerUtilities.GetVssConnection(_executionContext)) + { + var featureFlagService = _executionContext.GetHostContext().GetService(); + featureFlagService.InitializeFeatureService(_executionContext, connection); + _calculateTestRunSummary = featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); + _isFlakyCheckEnabled = featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); + } + } } } diff --git a/src/Agent.Worker/TestResults/TestRunDataPublisherHelper.cs b/src/Agent.Worker/TestResults/TestRunDataPublisherHelper.cs index 9dc42d00ae..0ebf89a02b 100644 --- a/src/Agent.Worker/TestResults/TestRunDataPublisherHelper.cs +++ b/src/Agent.Worker/TestResults/TestRunDataPublisherHelper.cs @@ -132,7 +132,7 @@ internal protected virtual CancellationToken GetCancellationToken() return new CancellationToken(); } - protected internal virtual bool GetFeatureFlagState(string featureFlagName, Guid serviceInstaceGuid) + protected internal virtual bool GetFeatureFlagState(string featureFlagName, Guid serviceInstanceGuid) { var featureFlagValue = false; @@ -140,7 +140,7 @@ protected internal virtual bool GetFeatureFlagState(string featureFlagName, Guid { var featureFlagService = _executionContext.GetHostContext().GetService(); featureFlagService.InitializeFeatureService(_executionContext, connection); - featureFlagValue = featureFlagService.GetFeatureFlagState(featureFlagName, serviceInstaceGuid); + featureFlagValue = featureFlagService.GetFeatureFlagState(featureFlagName, serviceInstanceGuid); } return featureFlagValue; From 6554ee6c4389c7c9b8fb9bb1c8a93c8d80349ee3 Mon Sep 17 00:00:00 2001 From: "Durga Vijay Prabhakar Ogirala (from Dev Box)" Date: Wed, 29 May 2024 10:11:52 +0530 Subject: [PATCH 2/3] Changes to adding in a new method without local variable usage for vssconn --- .../Legacy/LegacyTestRunDataPublisher.cs | 5 ++--- .../TestResults/ResultsCommandExtension.cs | 7 +++--- .../TestResults/TestDataPublisher.cs | 5 ++--- .../TestResults/Utils/FeatureFlagService.cs | 22 +++++++++++++++++++ 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs b/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs index f53d39bea9..57879529e2 100644 --- a/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs +++ b/src/Agent.Worker/TestResults/Legacy/LegacyTestRunDataPublisher.cs @@ -409,9 +409,8 @@ private void LoadFeatureFlagState() using (var connection = WorkerUtilities.GetVssConnection(_executionContext)) { var featureFlagService = _executionContext.GetHostContext().GetService(); - featureFlagService.InitializeFeatureService(_executionContext, connection); - _calculateTestRunSummary = featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); - _isFlakyCheckEnabled = featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); + _calculateTestRunSummary = featureFlagService.GetFeatureFlagStateByName(_executionContext, TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid, connection); + _isFlakyCheckEnabled = featureFlagService.GetFeatureFlagStateByName(_executionContext, TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid, connection); } } } diff --git a/src/Agent.Worker/TestResults/ResultsCommandExtension.cs b/src/Agent.Worker/TestResults/ResultsCommandExtension.cs index 2aa8e5cfeb..f8a25961fe 100644 --- a/src/Agent.Worker/TestResults/ResultsCommandExtension.cs +++ b/src/Agent.Worker/TestResults/ResultsCommandExtension.cs @@ -442,10 +442,9 @@ private void LoadFeatureFlagState() using (var connection = WorkerUtilities.GetVssConnection(_executionContext)) { var featureFlagService = _executionContext.GetHostContext().GetService(); - featureFlagService.InitializeFeatureService(_executionContext, connection); - _publishTestResultsLibFeatureState = featureFlagService.GetFeatureFlagState(TestResultsConstants.UsePublishTestResultsLibFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); - _enableAzureTestPlanFeatureState = featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableAzureTestPlanTaskFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); - _triggerCoverageMergeJobFeatureState = featureFlagService.GetFeatureFlagState(CodeCoverageConstants.TriggerCoverageMergeJobFF, TestResultsConstants.TFSServiceInstanceGuid); + _publishTestResultsLibFeatureState = featureFlagService.GetFeatureFlagStateByName(_executionContext, TestResultsConstants.UsePublishTestResultsLibFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid, connection); + _enableAzureTestPlanFeatureState = featureFlagService.GetFeatureFlagStateByName(_executionContext, TestResultsConstants.EnableAzureTestPlanTaskFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid, connection); + _triggerCoverageMergeJobFeatureState = featureFlagService.GetFeatureFlagStateByName(_executionContext, CodeCoverageConstants.TriggerCoverageMergeJobFF, TestResultsConstants.TFSServiceInstanceGuid, connection); } } } diff --git a/src/Agent.Worker/TestResults/TestDataPublisher.cs b/src/Agent.Worker/TestResults/TestDataPublisher.cs index dfc1383c8d..bb3c9319ec 100644 --- a/src/Agent.Worker/TestResults/TestDataPublisher.cs +++ b/src/Agent.Worker/TestResults/TestDataPublisher.cs @@ -324,9 +324,8 @@ private void LoadFeatureFlagState() using (var connection = WorkerUtilities.GetVssConnection(_executionContext)) { var featureFlagService = _executionContext.GetHostContext().GetService(); - featureFlagService.InitializeFeatureService(_executionContext, connection); - _calculateTestRunSummary = featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid); - _isFlakyCheckEnabled = featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); + _calculateTestRunSummary = featureFlagService.GetFeatureFlagStateByName(_executionContext, TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid, connection); + _isFlakyCheckEnabled = featureFlagService.GetFeatureFlagStateByName(_executionContext, TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid, connection); } } } diff --git a/src/Agent.Worker/TestResults/Utils/FeatureFlagService.cs b/src/Agent.Worker/TestResults/Utils/FeatureFlagService.cs index ca4b890961..b633ecc2d2 100644 --- a/src/Agent.Worker/TestResults/Utils/FeatureFlagService.cs +++ b/src/Agent.Worker/TestResults/Utils/FeatureFlagService.cs @@ -16,6 +16,8 @@ public interface IFeatureFlagService : IAgentService void InitializeFeatureService(IExecutionContext executionContext, VssConnection connection); bool GetFeatureFlagState(string featureFlagName, Guid serviceInstanceId); + + bool GetFeatureFlagStateByName(IExecutionContext executionContext, string featureFlagName, Guid serviceInstanceId, VssConnection connection); } public class FeatureFlagService : AgentService, IFeatureFlagService @@ -50,5 +52,25 @@ public bool GetFeatureFlagState(string featureFlagName, Guid serviceInstanceId) } return false; } + + public bool GetFeatureFlagStateByName(IExecutionContext executionContext, string featureFlagName, Guid serviceInstanceId, VssConnection connection) + { + try + { + FeatureAvailabilityHttpClient featureAvailabilityHttpClient = connection.GetClient(serviceInstanceId); + var featureFlag = featureAvailabilityHttpClient?.GetFeatureFlagByNameAsync(featureFlagName).Result; + if (featureFlag != null && featureFlag.EffectiveState.Equals("On", StringComparison.OrdinalIgnoreCase)) + { + executionContext.Debug(StringUtil.Format("{0} is on", featureFlagName)); + return true; + } + executionContext.Debug(StringUtil.Format("{0} is off", featureFlagName)); + } + catch + { + executionContext.Debug(StringUtil.Format("Failed to get FF {0} Value.", featureFlagName)); + } + return false; + } } } \ No newline at end of file From c79b9311b849f13bebff830f210b5a819af06f38 Mon Sep 17 00:00:00 2001 From: "Durga Vijay Prabhakar Ogirala (from Dev Box)" Date: Thu, 30 May 2024 13:40:57 +0530 Subject: [PATCH 3/3] reverting a nit chage --- src/Agent.Worker/TestResults/TestDataPublisher.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Agent.Worker/TestResults/TestDataPublisher.cs b/src/Agent.Worker/TestResults/TestDataPublisher.cs index bb3c9319ec..a617b007a6 100644 --- a/src/Agent.Worker/TestResults/TestDataPublisher.cs +++ b/src/Agent.Worker/TestResults/TestDataPublisher.cs @@ -38,6 +38,7 @@ public sealed class TestDataPublisher : AgentService, ITestDataPublisher private IParser _parser; private VssConnection _connection; + private IFeatureFlagService _featureFlagService; private string _testRunner; private bool _calculateTestRunSummary; private bool _isFlakyCheckEnabled; @@ -56,6 +57,8 @@ public void InitializePublisher(IExecutionContext context, string projectName, V _testResultsServer = HostContext.GetService(); _testResultsServer.InitializeServer(connection, _executionContext); var extensionManager = HostContext.GetService(); + _featureFlagService = HostContext.GetService(); + _featureFlagService.InitializeFeatureService(_executionContext, connection); _parser = (extensionManager.GetExtensions()).FirstOrDefault(x => _testRunner.Equals(x.Name, StringComparison.OrdinalIgnoreCase)); _testRunPublisherHelper = new TestRunDataPublisherHelper(_executionContext, _testRunPublisher, null, _testResultsServer); LoadFeatureFlagState();