diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/DiscoveryDataAggregator.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/DiscoveryDataAggregator.cs index 581c5b058d..11ea0294a7 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/DiscoveryDataAggregator.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/DiscoveryDataAggregator.cs @@ -191,34 +191,39 @@ public void MarkSourcesWithStatus(IEnumerable? sources, DiscoveryStatus continue; } - _sourcesWithDiscoveryStatus.AddOrUpdate(source, - _ => + MarkSourceWithStatus(source, status); + } + } + + private void MarkSourceWithStatus(string source, DiscoveryStatus status) + { + _sourcesWithDiscoveryStatus.AddOrUpdate(source, + _ => + { + if (status != DiscoveryStatus.NotDiscovered) { - if (status != DiscoveryStatus.NotDiscovered) - { - EqtTrace.Warning($"DiscoveryDataAggregator.MarkSourcesWithStatus: Undiscovered {source} added with status: '{status}'."); - } - else - { - EqtTrace.Verbose($"DiscoveryDataAggregator.MarkSourcesWithStatus: Adding {source} with status: '{status}'."); - } - - return status; - }, - (_, previousStatus) => + EqtTrace.Warning($"DiscoveryDataAggregator.MarkSourceWithStatus: Undiscovered {source} added with status: '{status}'."); + } + else { - if (previousStatus == DiscoveryStatus.FullyDiscovered && status != DiscoveryStatus.FullyDiscovered - || previousStatus == DiscoveryStatus.PartiallyDiscovered && (status == DiscoveryStatus.NotDiscovered || status == DiscoveryStatus.SkippedDiscovery)) - { - EqtTrace.Warning($"DiscoveryDataAggregator.MarkSourcesWithStatus: Downgrading source {source} status from '{previousStatus}' to '{status}'."); - } - else if (previousStatus != status) - { - EqtTrace.Verbose($"DiscoveryDataAggregator.MarkSourcesWithStatus: Upgrading {source} status from '{previousStatus}' to '{status}'."); - } - return status; - }); - } + EqtTrace.Verbose($"DiscoveryDataAggregator.MarkSourceWithStatus: Adding {source} with status: '{status}'."); + } + + return status; + }, + (_, previousStatus) => + { + if (previousStatus == DiscoveryStatus.FullyDiscovered && status != DiscoveryStatus.FullyDiscovered + || previousStatus == DiscoveryStatus.PartiallyDiscovered && (status == DiscoveryStatus.NotDiscovered || status == DiscoveryStatus.SkippedDiscovery)) + { + EqtTrace.Warning($"DiscoveryDataAggregator.MarkSourceWithStatus: Downgrading source {source} status from '{previousStatus}' to '{status}'."); + } + else if (previousStatus != status) + { + EqtTrace.Verbose($"DiscoveryDataAggregator.MarkSourceWithStatus: Upgrading {source} status from '{previousStatus}' to '{status}'."); + } + return status; + }); } /// @@ -245,6 +250,12 @@ public void MarkSourcesWithStatus(IEnumerable? sources, DiscoveryStatus foreach (var testCase in testCases) { + if (_isMessageSent == 1) + { + EqtTrace.Verbose("DiscoveryDataAggregator.MarkSourcesBasedOnDiscoveredTestCases: Message was already sent so skipping remaining source updates."); + return previousSource; + } + var currentSource = testCase.Source; // We rely on the fact that sources are processed in a sequential way, which @@ -252,13 +263,13 @@ public void MarkSourcesWithStatus(IEnumerable? sources, DiscoveryStatus // assume that the previous source was fully discovered. if (previousSource is null || previousSource == currentSource) { - MarkSourcesWithStatus(new[] { currentSource }, DiscoveryStatus.PartiallyDiscovered); + MarkSourceWithStatus(currentSource, DiscoveryStatus.PartiallyDiscovered); } else if (currentSource != previousSource) { EqtTrace.Verbose($"DiscoveryDataAggregator.MarkSourcesBasedOnDiscoveredTestCases: Discovered test source changed from {previousSource} to {currentSource}."); - MarkSourcesWithStatus(new[] { previousSource }, DiscoveryStatus.FullyDiscovered); - MarkSourcesWithStatus(new[] { currentSource }, DiscoveryStatus.PartiallyDiscovered); + MarkSourceWithStatus(previousSource, DiscoveryStatus.FullyDiscovered); + MarkSourceWithStatus(currentSource, DiscoveryStatus.PartiallyDiscovered); } previousSource = currentSource; diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/DiscoveryDataAggregatorRegressionTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/DiscoveryDataAggregatorRegressionTests.cs index ffd5ba683a..56939b2738 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/DiscoveryDataAggregatorRegressionTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/DiscoveryDataAggregatorRegressionTests.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -125,4 +126,25 @@ public void MarkSourcesWithStatus_AfterMessageSent_ShouldSkipUpdate() var result = aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered); Assert.HasCount(1, result); } + + // Pins the _isMessageSent guard for MarkSourcesBasedOnDiscoveredTestCases + [TestMethod] + public void MarkSourcesBasedOnDiscoveredTestCases_AfterMessageSent_ShouldSkipUpdate() + { + var aggregator = new DiscoveryDataAggregator(); + aggregator.MarkSourcesWithStatus(new[] { "test.dll" }, DiscoveryStatus.NotDiscovered); + + // Mark message as sent before the batch arrives + aggregator.TryAggregateIsMessageSent(); + + // This batch should be entirely skipped since message was already sent + var testCases = new[] { new TestCase { Source = "test.dll" } }; + aggregator.MarkSourcesBasedOnDiscoveredTestCases(null, testCases); + + // Status should still be NotDiscovered, not PartiallyDiscovered + var notDiscovered = aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered); + var partiallyDiscovered = aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered); + Assert.HasCount(1, notDiscovered); + Assert.IsEmpty(partiallyDiscovered); + } }