diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryDataAggregator.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryDataAggregator.cs index 541d70b067..0089eb0e9d 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryDataAggregator.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryDataAggregator.cs @@ -72,7 +72,7 @@ public IDictionary GetAggregatedDiscoveryDataMetrics() /// Aggregate discovery data /// Must be thread-safe as this is expected to be called by parallel managers /// - public void Aggregate(DiscoveryCompleteEventArgs discoveryCompleteEventArgs) + public void Aggregate(DiscoveryCompleteEventArgs discoveryCompleteEventArgs!!) { lock (_dataUpdateSyncObject) { @@ -93,7 +93,7 @@ public void Aggregate(DiscoveryCompleteEventArgs discoveryCompleteEventArgs) AggregateDiscoveryDataMetrics(discoveryCompleteEventArgs.Metrics); } - internal /* for testing purposes */ void AggregateDiscoveryStatus( + private void AggregateDiscoveryStatus( IEnumerable notDiscoveredSources, IEnumerable partiallyDiscoveredSources, IEnumerable fullyDiscoveredSources) @@ -143,7 +143,7 @@ public void Aggregate(DiscoveryCompleteEventArgs discoveryCompleteEventArgs) /// Aggregates the metrics from Test Host Process. /// /// - internal /* for testing purposes */ void AggregateDiscoveryDataMetrics(IDictionary? metrics) + private void AggregateDiscoveryDataMetrics(IDictionary? metrics) { if (metrics == null || metrics.Count == 0 || _metricsAggregator == null) { diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryEventsHandler.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryEventsHandler.cs index 09ae49f925..07554b341d 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryEventsHandler.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelDiscoveryEventsHandler.cs @@ -62,10 +62,13 @@ public void HandleDiscoveryComplete(DiscoveryCompleteEventArgs discoveryComplete // Aggregate data for final discovery complete _discoveryDataAggregator.Aggregate(discoveryCompleteEventArgs); - // we get discovery complete events from each host process - // so we cannot "complete" the actual operation until all sources are consumed - // We should not block last chunk results while we aggregate overall discovery data - if (lastChunk?.Any() == true) + // We get DiscoveryComplete events from each ProxyDiscoveryManager (each testhost) and + // they contain the last chunk of tests that were discovered in that particular testhost. + // We want to send them to the IDE, but we don't want to tell it that discovery finished, + // because other sources are still being discovered. So we translate the last chunk to a + // normal TestsDiscovered event and send that to the IDE. Once all sources are completed, + // we will send a single DiscoveryComplete. + if (lastChunk != null) { ConvertToRawMessageAndSend(MessageType.TestCasesFound, lastChunk); HandleDiscoveredTests(lastChunk); @@ -76,15 +79,16 @@ public void HandleDiscoveryComplete(DiscoveryCompleteEventArgs discoveryComplete var parallelDiscoveryComplete = _parallelProxyDiscoveryManager.HandlePartialDiscoveryComplete( _proxyDiscoveryManager, discoveryCompleteEventArgs.TotalCount, - null, // lastChunk should be null as we already sent this data above + null, // Set lastChunk to null, because we already sent the data using HandleDiscoveredTests above. discoveryCompleteEventArgs.IsAborted); + if (!parallelDiscoveryComplete) { return; } - // As we immediately return results to IDE in case of aborting - // we need to set isAborted = true and totalTests = -1 + // As we immediately return results to IDE in case of aborting we need to set + // isAborted = true and totalTests = -1 if (_parallelProxyDiscoveryManager.IsAbortRequested) { _discoveryDataAggregator.Aggregate(new(-1, true)); diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs index 83138ccc1e..15eec9d031 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs @@ -235,12 +235,9 @@ public void Close() /// public void HandleDiscoveryComplete(DiscoveryCompleteEventArgs discoveryCompleteEventArgs, IEnumerable lastChunk) { - if (lastChunk != null) - { - // When discovery is complete then the last discovered source is still marked - // as partially discovered, so we need to mark it as fully discovered. - _discoverySourceStatusCache.MarkTheLastChunkSourcesAsFullyDiscovered(lastChunk); - } + // When discovery is complete then the last discovered source is still marked + // as partially discovered, so we need to mark it as fully discovered. + _discoverySourceStatusCache.MarkTheLastChunkSourcesAsFullyDiscovered(lastChunk); _baseTestDiscoveryEventsHandler.HandleDiscoveryComplete( new(discoveryCompleteEventArgs.TotalCount, discoveryCompleteEventArgs.IsAborted) @@ -249,8 +246,7 @@ public void HandleDiscoveryComplete(DiscoveryCompleteEventArgs discoveryComplete NotDiscoveredSources = _discoverySourceStatusCache.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered), PartiallyDiscoveredSources = _discoverySourceStatusCache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered), FullyDiscoveredSources = _discoverySourceStatusCache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered), - } - , lastChunk); + }, lastChunk); } /// diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryManager.cs index b96ac31fa2..de0e9e03ad 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryManager.cs @@ -39,7 +39,7 @@ public class DiscoveryManager : IDiscoveryManager private DiscoveryCriteria _discoveryCriteria; private readonly CancellationTokenSource _cancellationTokenSource = new(); private readonly DiscoverySourceStatusCache _discoverySourceStatusCache = new(); - + /// /// Initializes a new instance of the class. /// diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoverySourceStatusCache.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoverySourceStatusCache.cs index 9ddc8ab656..befc6ed168 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoverySourceStatusCache.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoverySourceStatusCache.cs @@ -16,7 +16,7 @@ internal class DiscoverySourceStatusCache private string? _previousSource; - public void MarkSourcesWithStatus(IEnumerable sources, DiscoveryStatus status) + public void MarkSourcesWithStatus(IEnumerable? sources, DiscoveryStatus status) { if (sources is null) { @@ -40,27 +40,33 @@ public void MarkSourcesWithStatus(IEnumerable sources, DiscoveryStatus return status; }, - (_, _) => + (_, previousStatus) => { + if (previousStatus == DiscoveryStatus.FullyDiscovered && status != DiscoveryStatus.FullyDiscovered + || previousStatus == DiscoveryStatus.PartiallyDiscovered && status == DiscoveryStatus.NotDiscovered) + { + EqtTrace.Warning($"DiscoverySourceStatusCache.MarkSourcesWithStatus: Downgrading source status from {previousStatus} to {status}."); + } + EqtTrace.Info($"DiscoverySourceStatusCache.MarkSourcesWithStatus: Marking {source} with {status} status."); return status; }); } } - public void MarkTheLastChunkSourcesAsFullyDiscovered(IEnumerable lastChunk) + public void MarkTheLastChunkSourcesAsFullyDiscovered(IEnumerable? lastChunk) { // When all testcases count in project is dividable by chunk size (e.g. 100 tests and // chunk size of 10) then lastChunk is coming as empty. In this case, we need to take // the lastSource and mark it as FullyDiscovered. - IEnumerable lastChunkSources = lastChunk.Any() + IEnumerable lastChunkSources = lastChunk?.Any() == true ? lastChunk.Select(testcase => testcase.Source) : new[] { _previousSource }; MarkSourcesWithStatus(lastChunkSources, DiscoveryStatus.FullyDiscovered); } - public void MarkSourcesBasedOnDiscoveredTestCases(IEnumerable testCases) + public void MarkSourcesBasedOnDiscoveredTestCases(IEnumerable? testCases) { if (testCases is null) { @@ -92,7 +98,7 @@ public List GetSourcesWithStatus(DiscoveryStatus discoveryStatus) => GetSourcesWithStatus(discoveryStatus, _sourcesWithDiscoveryStatus); public static List GetSourcesWithStatus(DiscoveryStatus discoveryStatus, - ConcurrentDictionary sourcesWithDiscoveryStatus) + ConcurrentDictionary? sourcesWithDiscoveryStatus) { // If by some accident SourcesWithDiscoveryStatus map is empty we will return empty list return sourcesWithDiscoveryStatus is null || sourcesWithDiscoveryStatus.IsEmpty diff --git a/src/Microsoft.TestPlatform.ObjectModel/Client/Events/DiscoveryCompleteEventArgs.cs b/src/Microsoft.TestPlatform.ObjectModel/Client/Events/DiscoveryCompleteEventArgs.cs index 95547d7a22..c0d7d473e1 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Client/Events/DiscoveryCompleteEventArgs.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Client/Events/DiscoveryCompleteEventArgs.cs @@ -27,11 +27,6 @@ public DiscoveryCompleteEventArgs(long totalTests, bool isAborted, IList partiallyDiscoveredSources, IList notDiscoveredSources) { - // This event is always raised from the client side, while the total count of tests is maintained - // only at the testhost end. In case of a discovery abort (various reasons including crash), it is - // not possible to get a list of total tests from testhost. Hence we enforce a -1 count. - Debug.Assert((!isAborted || -1 == totalTests), "If discovery request is aborted totalTest should be -1."); - TotalCount = totalTests; IsAborted = isAborted; diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelDiscoveryDataAggregatorTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelDiscoveryDataAggregatorTests.cs index 383dbdfe26..064ec65d86 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelDiscoveryDataAggregatorTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelDiscoveryDataAggregatorTests.cs @@ -7,6 +7,7 @@ using Microsoft.VisualStudio.TestPlatform.Common.Telemetry; using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -51,7 +52,7 @@ public void AggregateDiscoveryDataMetricsShouldAggregateMetricsCorrectly() { var aggregator = new ParallelDiscoveryDataAggregator(); - aggregator.AggregateDiscoveryDataMetrics(null); + aggregator.Aggregate(new(0, false) { Metrics = null }); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); Assert.AreEqual(0, runMetrics.Count); @@ -62,13 +63,16 @@ public void AggregateDiscoveryDataMetricsShouldAddTotalTestsDiscovered() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.TotalTestsDiscovered, 2 } + Metrics = new Dictionary + { + { TelemetryDataConstants.TotalTestsDiscovered, 2 }, + } }; - aggregator.AggregateDiscoveryDataMetrics(dict); - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); @@ -81,13 +85,16 @@ public void AggregateDiscoveryDataMetricsShouldAddTimeTakenToDiscoverTests() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter, .02091 } + Metrics = new Dictionary + { + { TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter, .02091 } + }, }; - aggregator.AggregateDiscoveryDataMetrics(dict); - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); @@ -100,13 +107,16 @@ public void AggregateDiscoveryDataMetricsShouldAddTimeTakenByAllAdapters() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.TimeTakenInSecByAllAdapters, .02091 } + Metrics = new Dictionary + { + { TelemetryDataConstants.TimeTakenInSecByAllAdapters, .02091 } + }, }; - aggregator.AggregateDiscoveryDataMetrics(dict); - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); @@ -119,13 +129,16 @@ public void AggregateDiscoveryDataMetricsShouldAddTimeTakenToLoadAdapters() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.TimeTakenToLoadAdaptersInSec, .02091 } + Metrics = new Dictionary + { + { TelemetryDataConstants.TimeTakenToLoadAdaptersInSec, .02091 } + }, }; - aggregator.AggregateDiscoveryDataMetrics(dict); - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); @@ -138,13 +151,16 @@ public void AggregateDiscoveryDataMetricsShouldNotAggregateDiscoveryState() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.DiscoveryState, "Completed" } + Metrics = new Dictionary + { + { TelemetryDataConstants.DiscoveryState, "Completed" } + }, }; - aggregator.AggregateDiscoveryDataMetrics(dict); - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); Assert.IsFalse(runMetrics.TryGetValue(TelemetryDataConstants.DiscoveryState, out _)); @@ -155,9 +171,12 @@ public void GetAggregatedDiscoveryDataMetricsShouldReturnEmptyIfMetricAggregator { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary(); + var args = new DiscoveryCompleteEventArgs(0, false) + { + Metrics = new Dictionary(), + }; - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); Assert.AreEqual(0, runMetrics.Count); @@ -167,9 +186,13 @@ public void GetAggregatedDiscoveryDataMetricsShouldReturnEmptyIfMetricAggregator public void GetAggregatedDiscoveryDataMetricsShouldReturnEmptyIfMetricsIsNull() { var aggregator = new ParallelDiscoveryDataAggregator(); - _ = new Dictionary(); - aggregator.AggregateDiscoveryDataMetrics(null); + var args = new DiscoveryCompleteEventArgs(0, false) + { + Metrics = null, + }; + + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); Assert.AreEqual(0, runMetrics.Count); @@ -180,13 +203,16 @@ public void GetDiscoveryDataMetricsShouldAddTotalAdaptersUsedIfMetricsIsNotEmpty { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.TotalTestsByAdapter, 2 } + Metrics = new Dictionary + { + { TelemetryDataConstants.TotalTestsByAdapter, 2 } + }, }; - aggregator.AggregateDiscoveryDataMetrics(dict); - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); @@ -199,13 +225,16 @@ public void GetDiscoveryDataMetricsShouldAddNumberOfAdapterDiscoveredIfMetricsIs { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary + var args = new DiscoveryCompleteEventArgs(0, false) { - { TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter + "executor:MSTestV1", .02091 }, - { TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter + "executor:MSTestV2", .02091 } + Metrics = new Dictionary + { + { TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter + "executor:MSTestV1", .02091 }, + { TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter + "executor:MSTestV2", .02091 }, + }, }; - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); @@ -217,9 +246,13 @@ public void GetDiscoveryDataMetricsShouldAddNumberOfAdapterDiscoveredIfMetricsIs public void GetDiscoveryDataMetricsShouldNotAddTotalAdaptersUsedIfMetricsIsEmpty() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary(); - aggregator.AggregateDiscoveryDataMetrics(dict); + var args = new DiscoveryCompleteEventArgs(0, false) + { + Metrics = new Dictionary(), + }; + + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); Assert.IsFalse(runMetrics.TryGetValue(TelemetryDataConstants.NumberOfAdapterUsedToDiscoverTests, out _)); @@ -229,9 +262,12 @@ public void GetDiscoveryDataMetricsShouldNotAddTotalAdaptersUsedIfMetricsIsEmpty public void GetDiscoveryDataMetricsShouldNotAddNumberOfAdapterDiscoveredIfMetricsIsEmpty() { var aggregator = new ParallelDiscoveryDataAggregator(); - var dict = new Dictionary(); + var args = new DiscoveryCompleteEventArgs(0, false) + { + Metrics = new Dictionary(), + }; - aggregator.AggregateDiscoveryDataMetrics(dict); + aggregator.Aggregate(args); var runMetrics = aggregator.GetAggregatedDiscoveryDataMetrics(); Assert.IsFalse(runMetrics.TryGetValue(TelemetryDataConstants.NumberOfAdapterDiscoveredDuringDiscovery, out _)); @@ -253,64 +289,89 @@ public void AggregateShouldAggregateMessageSentCorrectly() } [TestMethod] - public void AggregateShouldAggregateSourcesCorrectly() + public void AggregateDiscoveryStatusChangeStatusOfCorrectSource() { // Arrange var aggregator = new ParallelDiscoveryDataAggregator(); - var sources = new List() { "sample.dll" }; + var sources = new List { "a", }; // Act - aggregator.MarkSourcesWithStatus(sources, DiscoveryStatus.NotDiscovered); - var sourcesWithNotDiscoveredStatus = aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered); + aggregator.Aggregate(new(0, false) + { + NotDiscoveredSources = sources, + }); // Assert - Assert.AreEqual(1, sourcesWithNotDiscoveredStatus.Count); + Assert.AreEqual(1, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered).Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); // Act - aggregator.MarkSourcesWithStatus(sources, DiscoveryStatus.FullyDiscovered); - var sourcesWithFullyDiscoveryStatus = aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered); + aggregator.Aggregate(new(0, false) + { + PartiallyDiscoveredSources = sources, + }); // Assert - Assert.AreEqual(1, sourcesWithFullyDiscoveryStatus.Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered).Count); + Assert.AreEqual(1, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); + + // Act + aggregator.Aggregate(new(0, false) + { + FullyDiscoveredSources = sources, + }); + + // Assert + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered).Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + Assert.AreEqual(1, aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); } [TestMethod] - public void AggregateDiscoveryStatusHandlesNotDiscoveredSources() + public void AggregateDiscoveryStatusAggregatesNotDiscoveredSources() { // Arrange var aggregator = new ParallelDiscoveryDataAggregator(); - aggregator.MarkSourcesWithStatus(new List { "a", "b", "d" }, DiscoveryStatus.NotDiscovered); + aggregator.Aggregate(new(0, false) + { + NotDiscoveredSources = new List { "a", "b", "d" }, + }); // Act - aggregator.AggregateDiscoveryStatus( - new List { "a", "c", "d" }, - Enumerable.Empty(), - Enumerable.Empty()); + aggregator.Aggregate(new(0, false) + { + NotDiscoveredSources = new List { "a", "c", "d" }, + }); // Assert CollectionAssert.AreEquivalent( new List { "a", "b", "c", "d" }, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered)); - Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered)); - Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered)); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); } [TestMethod] - public void AggregateDiscoveryStatusHandlesPartiallyDiscoveredSources() + public void AggregateDiscoveryStatusAggregatesPartiallyDiscoveredSources() { // Arrange var aggregator = new ParallelDiscoveryDataAggregator(); - aggregator.MarkSourcesWithStatus(new List { "a", "d" }, DiscoveryStatus.NotDiscovered); - aggregator.MarkSourcesWithStatus(new List { "b" }, DiscoveryStatus.FullyDiscovered); + aggregator.Aggregate(new(0, false) + { + NotDiscoveredSources = new List { "a", "d" }, + FullyDiscoveredSources = new List { "b" }, + }); // Act - aggregator.AggregateDiscoveryStatus( - Enumerable.Empty(), - new List { "a", "b", "c", "d" }, - Enumerable.Empty()); + aggregator.Aggregate(new(0, false) + { + PartiallyDiscoveredSources = new List { "a", "b", "c", "d" }, + }); // Assert - Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered)); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered).Count); CollectionAssert.AreEquivalent( new List { "a", "c", "d" }, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered)); @@ -320,23 +381,26 @@ public void AggregateDiscoveryStatusHandlesPartiallyDiscoveredSources() } [TestMethod] - public void AggregateDiscoveryStatusHandlesFullyDiscoveredSources() + public void AggregateDiscoveryStatusAggregatesFullyDiscoveredSources() { // Arrange var aggregator = new ParallelDiscoveryDataAggregator(); - aggregator.MarkSourcesWithStatus(new List { "a", "d" }, DiscoveryStatus.NotDiscovered); - aggregator.MarkSourcesWithStatus(new List { "b" }, DiscoveryStatus.PartiallyDiscovered); - aggregator.MarkSourcesWithStatus(new List { "e" }, DiscoveryStatus.FullyDiscovered); + aggregator.Aggregate(new(0, false) + { + NotDiscoveredSources = new List { "a", "d" }, + PartiallyDiscoveredSources = new List { "b" }, + FullyDiscoveredSources = new List { "e" }, + }); // Act - aggregator.AggregateDiscoveryStatus( - Enumerable.Empty(), - Enumerable.Empty(), - new List { "a", "b", "c", "d", "e" }); + aggregator.Aggregate(new(0, false) + { + FullyDiscoveredSources = new List { "a", "b", "c", "d", "e" }, + }); // Assert - Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered)); - Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered)); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered).Count); + Assert.AreEqual(0, aggregator.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); CollectionAssert.AreEquivalent( new List { "a", "b", "c", "d", "e" }, aggregator.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered)); diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscoverySourceStatusCacheTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscoverySourceStatusCacheTests.cs new file mode 100644 index 0000000000..385bcc013b --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscoverySourceStatusCacheTests.cs @@ -0,0 +1,263 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; + +using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Discovery; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.TestPlatform.CrossPlatEngine.UnitTests.Discovery; + +[TestClass] +public class DiscoverySourceStatusCacheTests +{ + [DataTestMethod] + [DataRow(DiscoveryStatus.FullyDiscovered)] + [DataRow(DiscoveryStatus.PartiallyDiscovered)] + [DataRow(DiscoveryStatus.NotDiscovered)] + public void MarkSourcesWithStatusWhenSourcesIsNullDoesNothing(DiscoveryStatus discoveryStatus) + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + + // Act + cache.MarkSourcesWithStatus(null, discoveryStatus); + + // Assert + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered).Count); + } + + [DataTestMethod] + [DataRow(DiscoveryStatus.FullyDiscovered)] + [DataRow(DiscoveryStatus.PartiallyDiscovered)] + [DataRow(DiscoveryStatus.NotDiscovered)] + public void MarkSourcesWithStatusIgnoresNullSources(DiscoveryStatus discoveryStatus) + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + var sources = new[] { "a", null, "b" }; + + // Sanity check + Assert.AreEqual(0, cache.GetSourcesWithStatus(discoveryStatus).Count); + + // Act + cache.MarkSourcesWithStatus(sources, discoveryStatus); + + // Assert + CollectionAssert.AreEquivalent(new List { "a", "b" }, cache.GetSourcesWithStatus(discoveryStatus)); + } + + [DataTestMethod] + [DataRow(DiscoveryStatus.FullyDiscovered)] + [DataRow(DiscoveryStatus.PartiallyDiscovered)] + public void MarkSourcesWithStatusWhenSourceAddedAndStatusDifferentFromNotDiscoveredLogsWarning(DiscoveryStatus discoveryStatus) + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + + // Act + cache.MarkSourcesWithStatus(new[] { "a" }, discoveryStatus); + + // Assert + CollectionAssert.AreEquivalent(new List { "a" }, cache.GetSourcesWithStatus(discoveryStatus)); + } + + [DataTestMethod] + [DataRow(DiscoveryStatus.NotDiscovered)] + [DataRow(DiscoveryStatus.PartiallyDiscovered)] + public void MarkSourcesWithStatusWhenSourceStatusWasFullyDiscoveredAndIsDowngradedLogsWarning(DiscoveryStatus discoveryStatus) + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + cache.MarkSourcesWithStatus(new[] { "a" }, DiscoveryStatus.FullyDiscovered); + + // Act + cache.MarkSourcesWithStatus(new[] { "a" }, discoveryStatus); + + // Assert + CollectionAssert.AreEquivalent(new List { "a" }, cache.GetSourcesWithStatus(discoveryStatus)); + } + + [TestMethod] + public void MarkSourcesWithStatusWhenSourceStatusWasPartiallyDiscoveredAndIsDowngradedLogsWarning() + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + cache.MarkSourcesWithStatus(new[] { "a" }, DiscoveryStatus.PartiallyDiscovered); + + // Act + cache.MarkSourcesWithStatus(new[] { "a" }, DiscoveryStatus.NotDiscovered); + + // Assert + CollectionAssert.AreEquivalent(new List { "a" }, cache.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered)); + } + + [DataTestMethod] + [DataRow(true)] + [DataRow(false)] + public void MarkTheLastChunkSourcesAsFullyDiscoveredWhenLastChunkIsNullOrEmptyUsesPreviousSource(bool isEmpty) + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + cache.MarkSourcesBasedOnDiscoveredTestCases(new TestCase[] { new() { Source = "a" } }); + + // Sanity check + CollectionAssert.AreEquivalent(new List { "a" }, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered)); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); + + // Act + cache.MarkTheLastChunkSourcesAsFullyDiscovered(isEmpty ? Enumerable.Empty() : null); + + // Assert + CollectionAssert.AreEquivalent(new List { "a" }, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered)); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + } + + [TestMethod] + public void MarkTheLastChunkSourcesAsFullyDiscoveredMarkAllSourcesAsFullyDiscoveredRegardlessOfPreviousState() + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + cache.MarkSourcesWithStatus(new[] { "old" }, DiscoveryStatus.NotDiscovered); + var testCases = new TestCase[] + { + new() { Source = "a" }, + new() { Source = "b" }, + new() { Source = "c" }, + }; + + // Sanity check + CollectionAssert.AreEquivalent(new List { "old" }, cache.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered)); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered).Count); + + // Act + cache.MarkTheLastChunkSourcesAsFullyDiscovered(testCases); + + // Assert + CollectionAssert.AreEquivalent(new List { "old" }, cache.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered)); + Assert.AreEqual(0, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered).Count); + CollectionAssert.AreEquivalent(new List { "a", "b", "c", }, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered)); + } + + [TestMethod] + public void MarkSourcesBasedOnDiscoveredTestCasesHandlesTestCasesFromMultipleSources() + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + var testCases = new TestCase[] + { + new() { Source = "a" }, + new() { Source = "a" }, + new() { Source = "a" }, + new() { Source = "b" }, + new() { Source = "c" }, + }; + + // Act + cache.MarkSourcesBasedOnDiscoveredTestCases(testCases); + + // Assert + CollectionAssert.AreEquivalent(new List { "a", "b" }, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered)); + CollectionAssert.AreEquivalent(new List { "c" }, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered)); + } + [TestMethod] + public void MarkSourcesBasedOnDiscoveredTestCasesReuseLastDiscoveredSource() + { + // Arrange + var cache = new DiscoverySourceStatusCache(); + cache.MarkSourcesBasedOnDiscoveredTestCases(new TestCase[] { new() { Source = "a" } }); + + // Act + cache.MarkSourcesBasedOnDiscoveredTestCases(new TestCase[] { new() { Source = "b" }, }); + + // Assert + CollectionAssert.AreEquivalent(new List { "a" }, cache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered)); + CollectionAssert.AreEquivalent(new List { "b" }, cache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered)); + } + + [DataTestMethod] + [DataRow(DiscoveryStatus.FullyDiscovered)] + [DataRow(DiscoveryStatus.PartiallyDiscovered)] + [DataRow(DiscoveryStatus.NotDiscovered)] + public void GetSourcesWithStatusWhenEmptyDictionaryReturnsEmptyList(DiscoveryStatus discoveryStatus) + { + var instanceSources = new DiscoverySourceStatusCache().GetSourcesWithStatus(discoveryStatus); + Assert.AreEqual(0, instanceSources.Count); + + var givenDicSources = DiscoverySourceStatusCache.GetSourcesWithStatus(discoveryStatus, new()); + Assert.AreEqual(0, givenDicSources.Count); + } + + [DataTestMethod] + [DataRow(DiscoveryStatus.FullyDiscovered)] + [DataRow(DiscoveryStatus.PartiallyDiscovered)] + [DataRow(DiscoveryStatus.NotDiscovered)] + public void GetSourcesWithStatusWhenNullDictionaryReturnsEmptyList(DiscoveryStatus discoveryStatus) + { + var givenDicSources = DiscoverySourceStatusCache.GetSourcesWithStatus(discoveryStatus, null); + Assert.AreEqual(0, givenDicSources.Count); + } + + [TestMethod] + public void GetSourcesWithStatusCorrectlyFiltersSources() + { + // Arrange + var dic = new ConcurrentDictionary + { + ["fully1"] = DiscoveryStatus.FullyDiscovered, + ["fully2"] = DiscoveryStatus.FullyDiscovered, + ["fully3"] = DiscoveryStatus.FullyDiscovered, + ["fully4"] = DiscoveryStatus.FullyDiscovered, + ["fully5"] = DiscoveryStatus.FullyDiscovered, + + ["partially1"] = DiscoveryStatus.PartiallyDiscovered, + ["partially2"] = DiscoveryStatus.PartiallyDiscovered, + ["partially3"] = DiscoveryStatus.PartiallyDiscovered, + ["partially4"] = DiscoveryStatus.PartiallyDiscovered, + + ["not1"] = DiscoveryStatus.NotDiscovered, + ["not2"] = DiscoveryStatus.NotDiscovered, + ["not3"] = DiscoveryStatus.NotDiscovered, + }; + + // Act + var fullyDiscovered = DiscoverySourceStatusCache.GetSourcesWithStatus(DiscoveryStatus.FullyDiscovered, dic); + var partiallyDiscovered = DiscoverySourceStatusCache.GetSourcesWithStatus(DiscoveryStatus.PartiallyDiscovered, dic); + var notDiscovered = DiscoverySourceStatusCache.GetSourcesWithStatus(DiscoveryStatus.NotDiscovered, dic); + + // Assert + CollectionAssert.AreEquivalent( + new List + { + "fully1", + "fully2", + "fully3", + "fully4", + "fully5", + }, fullyDiscovered); + + CollectionAssert.AreEquivalent( + new List + { + "partially1", + "partially2", + "partially3", + "partially4", + }, partiallyDiscovered); + + CollectionAssert.AreEquivalent( + new List + { + "not1", + "not2", + "not3", + }, notDiscovered); + } +}