Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FilterOptions is not serialized correctly when running .NET Core tests #1551

Merged
merged 5 commits into from
Apr 20, 2018
Merged

FilterOptions is not serialized correctly when running .NET Core tests #1551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
12ae44e
Initial commit
abhishkk Apr 18, 2018
9b23de6
Passing Filter Options to each execution manager of parallal execution.
abhishkk Apr 19, 2018
cf40b44
Unit tests
abhishkk Apr 19, 2018
ed56df5
Merge branch 'master' into filterOptionsNull
abhishkk Apr 19, 2018
8a1b4d4
Merge branch 'master' into filterOptionsNull
abhishkk Apr 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,32 +287,15 @@ 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actualTestRunCriteria [](start = 85, length = 21)

how was parallel ever working?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When FilterOptions are used, it never works. otherwise with test case filter it does.

}
}
else
{
if (this.TryFetchNextSource(this.testCaseListEnumerator, out List<TestCase> 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);
}
}

Expand Down
76 changes: 62 additions & 14 deletions src/Microsoft.TestPlatform.ObjectModel/Client/TestRunCriteria.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,34 @@ public TestRunCriteria(IEnumerable<string> sources, long frequencyOfRunStatsChan

/// <summary>
/// Initializes a new instance of the <see cref="TestRunCriteria"/> class.
/// Create the TestRunCriteria for a test run
/// </summary>
/// <param name="sources">
/// Sources which contains tests that should be executed
/// </param>
/// <param name="baseTestRunCriteria">
/// The BaseTestRunCriteria
/// <param name="frequencyOfRunStatsChangeEvent">
/// Frequency of run stats event
/// </param>
public TestRunCriteria(IEnumerable<string> sources, BaseTestRunCriteria baseTestRunCriteria)
: base(baseTestRunCriteria)
/// <param name="keepAlive">
/// Whether the execution process should be kept alive after the run is finished or not.
/// </param>
/// <param name="testSettings">
/// Settings used for this run.
/// </param>
/// <param name="runStatsChangeEventTimeout">
/// Timeout that triggers sending results regardless of cache size.
/// </param>
/// <param name="testHostLauncher">
/// Test host launcher. If null then default will be used.
/// </param>
public TestRunCriteria(
IEnumerable<string> 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<string> ?? sources.ToArray();
ValidateArg.NotNullOrEmpty(testSources, "sources");

this.AdapterSourceMap = new Dictionary<string, IEnumerable<string>>();
this.AdapterSourceMap.Add(Constants.UnspecifiedAdapterPath, testSources);
}

/// <summary>
Expand All @@ -136,20 +148,55 @@ public TestRunCriteria(IEnumerable<string> sources, BaseTestRunCriteria baseTest
/// <param name="testHostLauncher">
/// Test host launcher. If null then default will be used.
/// </param>
/// <param name="testCaseFilter">
/// Test case filter.
/// </param>
/// <param name="filterOptions">
/// Filter options.
/// </param>
public TestRunCriteria(
IEnumerable<string> 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<string> ?? sources.ToList();
ValidateArg.NotNullOrEmpty(testSources, "sources");

this.AdapterSourceMap = new Dictionary<string, IEnumerable<string>>();
this.AdapterSourceMap.Add(Constants.UnspecifiedAdapterPath, testSources);

this.TestCaseFilter = testCaseFilter;
this.FilterOptions = filterOptions;
}

/// <summary>
/// Initializes a new instance of the <see cref="TestRunCriteria"/> class.
/// Create the TestRunCriteria for a test run
/// </summary>
/// <param name="sources">
/// Sources which contains tests that should be executed
/// </param>
/// <param name="testRunCriteria">
/// The TestRunCriteria
/// </param>
public TestRunCriteria(IEnumerable<string> sources, TestRunCriteria testRunCriteria)
: base(testRunCriteria)
{
var testSources = sources as IList<string> ?? sources.ToArray();
ValidateArg.NotNullOrEmpty(testSources, "sources");

this.AdapterSourceMap = new Dictionary<string, IEnumerable<string>>();
this.AdapterSourceMap.Add(Constants.UnspecifiedAdapterPath, testSources);

this.TestCaseFilter = testRunCriteria.testCaseFilter;
this.FilterOptions = testRunCriteria.filterOptions;

}

/// <summary>
Expand Down Expand Up @@ -350,7 +397,7 @@ public string TestCaseFilter
return this.testCaseFilter;
}

set
private set
{
if (value != null && !this.HasSpecificSources)
{
Expand All @@ -373,7 +420,7 @@ public FilterOptions FilterOptions
return this.filterOptions;
}

set
private set
{
if (value != null && !this.HasSpecificSources)
{
Expand Down Expand Up @@ -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);
}

/// <inheritdoc/>
Expand Down
6 changes: 3 additions & 3 deletions src/vstest.console/TestPlatformHelpers/TestRequestManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.TestPlatform.Common.UnitTests/Logging/InternalTestLoggerEventsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ public void RaiseDiscoveryMessageShouldThrowExceptionIfNullTestRunMessageEventAr
public void RaiseTestRunStartShouldThrowExceptionIfAlreadyDisposed()
{
var loggerEvents = GetDisposedLoggerEvents();
TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" };
TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null);
TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria);

Assert.ThrowsException<ObjectDisposedException>(() =>
Expand Down Expand Up @@ -499,7 +499,7 @@ public void RaiseTestRunStartShouldInvokeRegisteredEventHandler()
TestRunStartEventArgs receivedEventArgs = null;
EventWaitHandle waitHandle = new AutoResetEvent(false);

TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" };
TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"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.
Expand Down
16 changes: 1 addition & 15 deletions ...tPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public ParallelProxyExecutionManagerTests()
// Configure sources
this.sources = new List<string>() { "1.dll", "2.dll" };
this.processedSources = new List<string>();
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();
Expand Down Expand Up @@ -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);
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ public void HandleTestRunStartShouldInvokeTestRunStartHandlerOfLoggers()
counter = 0;
waitHandle.Reset();

TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" };
TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null);
TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria);

// setup TestLogger
Expand All @@ -477,7 +477,7 @@ public void HandleTestRunStartShouldNotInvokeTestRunStartHandlerOfLoggersIfDispo
counter = 0;
waitHandle.Reset();

TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10) { TestCaseFilter = "Name=Test1" };
TestRunCriteria testRunCriteria = new TestRunCriteria(new List<string> { @"x:dummy\foo.dll" }, 10, false, string.Empty, TimeSpan.MaxValue, null, "Name=Test1", null);
TestRunStartEventArgs testRunStartEventArgs = new TestRunStartEventArgs(testRunCriteria);

// setup TestLogger
Expand Down
30 changes: 2 additions & 28 deletions test/Microsoft.TestPlatform.ObjectModel.UnitTests/Client/TestRunCriteriaTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void ConstructorForSourcesShouldInitializeAdapterSourceMap()
public void ConstructorForSourcesWithBaseTestRunCriteriaShouldInitializeAdapterSourceMap()
{
var sources = new List<string> { "s1.dll", "s2.dll" };
var testRunCriteria = new TestRunCriteria(sources, new BaseTestRunCriteria(10));
var testRunCriteria = new TestRunCriteria(sources, new TestRunCriteria(new List<String> { "temp.dll" }, 10));

Assert.IsNotNull(testRunCriteria.AdapterSourceMap);
CollectionAssert.AreEqual(new List<string> { "_none_" }, testRunCriteria.AdapterSourceMap.Keys);
Expand Down Expand Up @@ -139,37 +139,11 @@ public void HasSpecificTestsReturnsFalseIfSourcesAreSpecified()

#region TestCaseFilter tests

[TestMethod]
public void TestCaseFilterSetterShouldThrowIftestCriteriaIsBasedOnTests()
{
var testRunCriteria =
new TestRunCriteria(
new List<TestCase> { 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<string> { "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);
}
Expand Down