From e8da744c5976f530bf597cdf57a98fa66911ea0a Mon Sep 17 00:00:00 2001 From: Sarabjot Singh Date: Fri, 20 Apr 2018 17:08:28 +0530 Subject: [PATCH] FilterOptions is not serialized correctly when running .NET Core tests (#1551) (#1557) * Initial commit * Passing Filter Options to each execution manager of parallal execution. * Unit tests --- .../Parallel/ParallelProxyExecutionManager.cs | 21 +---- .../Client/TestRunCriteria.cs | 76 +++++++++++++++---- .../TestPlatformHelpers/TestRequestManager.cs | 6 +- .../Logging/InternalTestLoggerEventsTests.cs | 4 +- .../ParallelProxyExecutionManagerTests.cs | 16 +--- .../TestLoggerManagerTests.cs | 4 +- .../Client/TestRunCriteriaTests.cs | 30 +------- 7 files changed, 74 insertions(+), 83 deletions(-) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyExecutionManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyExecutionManager.cs index 0eeb22ce27..314bee403f 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyExecutionManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyExecutionManager.cs @@ -287,17 +287,7 @@ private void StartTestRunOnConcurrentManager(IProxyExecutionManager proxyExecuti if (this.TryFetchNextSource(this.sourceEnumerator, out string nextSource)) { EqtTrace.Info("ProxyParallelExecutionManager: Triggering test run for next source: {0}", nextSource); - - testRunCriteria = new TestRunCriteria( - new[] { nextSource }, - this.actualTestRunCriteria.FrequencyOfRunStatsChangeEvent, - this.actualTestRunCriteria.KeepAlive, - this.actualTestRunCriteria.TestRunSettings, - this.actualTestRunCriteria.RunStatsChangeEventTimeout, - this.actualTestRunCriteria.TestHostLauncher) - { - TestCaseFilter = this.actualTestRunCriteria.TestCaseFilter - }; + testRunCriteria = new TestRunCriteria(new[] { nextSource }, this.actualTestRunCriteria); } } else @@ -305,14 +295,7 @@ private void StartTestRunOnConcurrentManager(IProxyExecutionManager proxyExecuti if (this.TryFetchNextSource(this.testCaseListEnumerator, out List nextSetOfTests)) { EqtTrace.Info("ProxyParallelExecutionManager: Triggering test run for next source: {0}", nextSetOfTests?.FirstOrDefault()?.Source); - - testRunCriteria = new TestRunCriteria( - nextSetOfTests, - this.actualTestRunCriteria.FrequencyOfRunStatsChangeEvent, - this.actualTestRunCriteria.KeepAlive, - this.actualTestRunCriteria.TestRunSettings, - this.actualTestRunCriteria.RunStatsChangeEventTimeout, - this.actualTestRunCriteria.TestHostLauncher); + testRunCriteria = new TestRunCriteria(nextSetOfTests, this.actualTestRunCriteria); } } diff --git a/src/Microsoft.TestPlatform.ObjectModel/Client/TestRunCriteria.cs b/src/Microsoft.TestPlatform.ObjectModel/Client/TestRunCriteria.cs index 6838d6eb8d..10e739f382 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Client/TestRunCriteria.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Client/TestRunCriteria.cs @@ -97,22 +97,34 @@ public TestRunCriteria(IEnumerable sources, long frequencyOfRunStatsChan /// /// Initializes a new instance of the class. - /// Create the TestRunCriteria for a test run /// /// /// Sources which contains tests that should be executed /// - /// - /// The BaseTestRunCriteria + /// + /// Frequency of run stats event /// - public TestRunCriteria(IEnumerable sources, BaseTestRunCriteria baseTestRunCriteria) - : base(baseTestRunCriteria) + /// + /// Whether the execution process should be kept alive after the run is finished or not. + /// + /// + /// Settings used for this run. + /// + /// + /// Timeout that triggers sending results regardless of cache size. + /// + /// + /// Test host launcher. If null then default will be used. + /// + public TestRunCriteria( + IEnumerable sources, + long frequencyOfRunStatsChangeEvent, + bool keepAlive, + string testSettings, + TimeSpan runStatsChangeEventTimeout, + ITestHostLauncher testHostLauncher) + : this(sources, frequencyOfRunStatsChangeEvent, keepAlive, testSettings, runStatsChangeEventTimeout, testHostLauncher, null, null) { - var testSources = sources as IList ?? sources.ToArray(); - ValidateArg.NotNullOrEmpty(testSources, "sources"); - - this.AdapterSourceMap = new Dictionary>(); - this.AdapterSourceMap.Add(Constants.UnspecifiedAdapterPath, testSources); } /// @@ -136,13 +148,21 @@ public TestRunCriteria(IEnumerable sources, BaseTestRunCriteria baseTest /// /// Test host launcher. If null then default will be used. /// + /// + /// Test case filter. + /// + /// + /// Filter options. + /// public TestRunCriteria( IEnumerable sources, long frequencyOfRunStatsChangeEvent, bool keepAlive, string testSettings, TimeSpan runStatsChangeEventTimeout, - ITestHostLauncher testHostLauncher) + ITestHostLauncher testHostLauncher, + string testCaseFilter, + FilterOptions filterOptions) : base(frequencyOfRunStatsChangeEvent, keepAlive, testSettings, runStatsChangeEventTimeout, testHostLauncher) { var testSources = sources as IList ?? sources.ToList(); @@ -150,6 +170,33 @@ public TestRunCriteria( this.AdapterSourceMap = new Dictionary>(); this.AdapterSourceMap.Add(Constants.UnspecifiedAdapterPath, testSources); + + this.TestCaseFilter = testCaseFilter; + this.FilterOptions = filterOptions; + } + + /// + /// Initializes a new instance of the class. + /// Create the TestRunCriteria for a test run + /// + /// + /// Sources which contains tests that should be executed + /// + /// + /// The TestRunCriteria + /// + public TestRunCriteria(IEnumerable sources, TestRunCriteria testRunCriteria) + : base(testRunCriteria) + { + var testSources = sources as IList ?? sources.ToArray(); + ValidateArg.NotNullOrEmpty(testSources, "sources"); + + this.AdapterSourceMap = new Dictionary>(); + this.AdapterSourceMap.Add(Constants.UnspecifiedAdapterPath, testSources); + + this.TestCaseFilter = testRunCriteria.testCaseFilter; + this.FilterOptions = testRunCriteria.filterOptions; + } /// @@ -350,7 +397,7 @@ public string TestCaseFilter return this.testCaseFilter; } - set + private set { if (value != null && !this.HasSpecificSources) { @@ -373,7 +420,7 @@ public FilterOptions FilterOptions return this.filterOptions; } - set + private set { if (value != null && !this.HasSpecificSources) { @@ -422,7 +469,8 @@ public override string ToString() protected bool Equals(TestRunCriteria other) { return base.Equals(other) - && string.Equals(this.testCaseFilter, other.testCaseFilter); + && string.Equals(this.TestCaseFilter, other.TestCaseFilter) + && string.Equals(this.FilterOptions, other.FilterOptions); } /// diff --git a/src/vstest.console/TestPlatformHelpers/TestRequestManager.cs b/src/vstest.console/TestPlatformHelpers/TestRequestManager.cs index 7aa8465f48..74837c6578 100644 --- a/src/vstest.console/TestPlatformHelpers/TestRequestManager.cs +++ b/src/vstest.console/TestPlatformHelpers/TestRequestManager.cs @@ -259,9 +259,9 @@ public void RunTests(TestRunRequestPayload testRunRequestPayload, ITestHostLaunc testRunRequestPayload.KeepAlive, runsettings, this.commandLineOptions.TestStatsEventTimeout, - testHostLauncher); - runCriteria.TestCaseFilter = testRunRequestPayload.TestPlatformOptions?.TestCaseFilter; - runCriteria.FilterOptions = testRunRequestPayload.TestPlatformOptions?.FilterOptions; + testHostLauncher, + testRunRequestPayload.TestPlatformOptions?.TestCaseFilter, + testRunRequestPayload.TestPlatformOptions?.FilterOptions); } else { diff --git a/test/Microsoft.TestPlatform.Common.UnitTests/Logging/InternalTestLoggerEventsTests.cs b/test/Microsoft.TestPlatform.Common.UnitTests/Logging/InternalTestLoggerEventsTests.cs index 5f0a9cfd19..db50c38874 100644 --- a/test/Microsoft.TestPlatform.Common.UnitTests/Logging/InternalTestLoggerEventsTests.cs +++ b/test/Microsoft.TestPlatform.Common.UnitTests/Logging/InternalTestLoggerEventsTests.cs @@ -464,7 +464,7 @@ public void RaiseDiscoveryMessageShouldThrowExceptionIfNullTestRunMessageEventAr public void RaiseTestRunStartShouldThrowExceptionIfAlreadyDisposed() { var loggerEvents = GetDisposedLoggerEvents(); - TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" }; + TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null); TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria); Assert.ThrowsException(() => @@ -499,7 +499,7 @@ public void RaiseTestRunStartShouldInvokeRegisteredEventHandler() TestRunStartEventArgs receivedEventArgs = null; EventWaitHandle waitHandle = new AutoResetEvent(false); - TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" }; + TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null); TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria); // Register for the test run start event. diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs index cd00248035..6392dc7394 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs @@ -65,7 +65,7 @@ public ParallelProxyExecutionManagerTests() // Configure sources this.sources = new List() { "1.dll", "2.dll" }; this.processedSources = new List(); - this.testRunCriteriaWithSources = new TestRunCriteria(sources, 100); + this.testRunCriteriaWithSources = new TestRunCriteria(sources, 100, false, string.Empty, TimeSpan.MaxValue, null, "Name~Test", new FilterOptions() { FilterRegEx = @"^[^\s\(]+" }); // Configure testcases this.testCases = CreateTestCases(); @@ -118,8 +118,6 @@ public void CancelShouldCallAllConcurrentManagersOnce() [TestMethod] public void StartTestRunShouldProcessAllSources() { - // Testcase filter should be passed to all parallel test run criteria. - this.testRunCriteriaWithSources.TestCaseFilter = "Name~Test"; var parallelExecutionManager = this.SetupExecutionManager(this.proxyManagerFunc, 2); parallelExecutionManager.StartTestRun(testRunCriteriaWithSources, this.mockHandler.Object); @@ -129,19 +127,7 @@ public void StartTestRunShouldProcessAllSources() AssertMissingAndDuplicateSources(processedSources); } - [TestMethod] - public void StartTestRunShouldProcessAllSources1() - { - // Testcase filter should be passed to all parallel test run criteria. - this.testRunCriteriaWithSources.TestCaseFilter = "Name~Test"; - var parallelExecutionManager = this.SetupExecutionManager(this.proxyManagerFunc, 2); - - parallelExecutionManager.StartTestRun(testRunCriteriaWithSources, this.mockHandler.Object); - Assert.IsTrue(this.executionCompleted.Wait(taskTimeout), "Test run not completed."); - Assert.AreEqual(this.sources.Count, processedSources.Count, "All Sources must be processed."); - AssertMissingAndDuplicateSources(processedSources); - } [TestMethod] public void StartTestRunShouldProcessAllTestCases() diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs index a0cf67ad67..664ebd249f 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs @@ -453,7 +453,7 @@ public void HandleTestRunStartShouldInvokeTestRunStartHandlerOfLoggers() counter = 0; waitHandle.Reset(); - TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" }; + TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null); TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria); // setup TestLogger @@ -477,7 +477,7 @@ public void HandleTestRunStartShouldNotInvokeTestRunStartHandlerOfLoggersIfDispo counter = 0; waitHandle.Reset(); - TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" }; + TestRunCriteria testRunCriteria = new TestRunCriteria(new List { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null); TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria); // setup TestLogger diff --git a/test/Microsoft.TestPlatform.ObjectModel.UnitTests/Client/TestRunCriteriaTests.cs b/test/Microsoft.TestPlatform.ObjectModel.UnitTests/Client/TestRunCriteriaTests.cs index fc72d08cfb..33eb6ec1fe 100644 --- a/test/Microsoft.TestPlatform.ObjectModel.UnitTests/Client/TestRunCriteriaTests.cs +++ b/test/Microsoft.TestPlatform.ObjectModel.UnitTests/Client/TestRunCriteriaTests.cs @@ -32,7 +32,7 @@ public void ConstructorForSourcesShouldInitializeAdapterSourceMap() public void ConstructorForSourcesWithBaseTestRunCriteriaShouldInitializeAdapterSourceMap() { var sources = new List { "s1.dll", "s2.dll" }; - var testRunCriteria = new TestRunCriteria(sources, new BaseTestRunCriteria(10)); + var testRunCriteria = new TestRunCriteria(sources, new TestRunCriteria(new List { "temp.dll" }, 10)); Assert.IsNotNull(testRunCriteria.AdapterSourceMap); CollectionAssert.AreEqual(new List { "_none_" }, testRunCriteria.AdapterSourceMap.Keys); @@ -139,37 +139,11 @@ public void HasSpecificTestsReturnsFalseIfSourcesAreSpecified() #region TestCaseFilter tests - [TestMethod] - public void TestCaseFilterSetterShouldThrowIftestCriteriaIsBasedOnTests() - { - var testRunCriteria = - new TestRunCriteria( - new List { new TestCase("A.C.M", new Uri("excutor://dummy"), "s.dll") }, - frequencyOfRunStatsChangeEvent: 10); - - var isExceptionthrown = false; - try - { - testRunCriteria.TestCaseFilter = "foo"; - } - catch (InvalidOperationException ex) - { - isExceptionthrown = true; - Assert.AreEqual( - "Cannot specify TestCaseFilter for specific tests run. FilterCriteria is only for run with sources.", - ex.Message); - } - - Assert.IsTrue(isExceptionthrown); - } - [TestMethod] public void TestCaseFilterSetterShouldSetFilterCriteriaForSources() { var sources = new List { "s1.dll", "s2.dll" }; - var testRunCriteria = new TestRunCriteria(sources, frequencyOfRunStatsChangeEvent: 10); - - testRunCriteria.TestCaseFilter = "foo"; + var testRunCriteria = new TestRunCriteria(sources, 10, false, string.Empty, TimeSpan.MaxValue, null, "foo", null); Assert.AreEqual("foo", testRunCriteria.TestCaseFilter); }