-
Notifications
You must be signed in to change notification settings - Fork 331
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.
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
Adding Acceptance Tests for Translation Layer #1254
Adding Acceptance Tests for Translation Layer #1254
Conversation
return vsConsoleWrapper; | ||
} | ||
|
||
public string GetDefaultRunSettings() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Require docs for public APIs.
public void RunSelectedTestsWithoutTestPlatformOptions() | ||
{ | ||
var sources = new List<string> | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DRY
@@ -22,7 +22,7 @@ public interface IVsTestConsoleWrapper | |||
/// <summary> | |||
/// Initialize the TestPlatform with Paths to extensions like adapters, loggers and any other extensions | |||
/// </summary> | |||
/// <param name="pathToAdditionalExtensions">Folder Paths to where extension DLLs are present</param> | |||
/// <param name="pathToAdditionalExtensions">Full Paths to where extension DLLs are present</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still can be mistaken for folder path, how about "Full paths to extension dlls"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Will change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
new TestPlatformOptions() { CollectMetrics = false }, | ||
this.discoveryEventHandler2); | ||
|
||
// Assert. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert for CollectMetrics=false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
this.discoveryEventHandler); | ||
|
||
// Assert. | ||
Assert.AreEqual(6, this.discoveryEventHandler.DiscoveredTestCases.Count); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for parallel scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not supported yet
Assert.AreEqual(TestOutcome.Passed, this.runEventHandler.TestResults.FirstOrDefault().Outcome); | ||
|
||
// Release builds optimize code, hence line numbers are different. | ||
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add source navigation test for discovery scenario too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add navigation assert for other adapter too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
[TestMethod] | ||
public void DiscoverTestsUsingEventHandler2AndBatchSize() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as DiscoverTestsUsingDiscoveryEventHandler2AndTelemetryOptedOut
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In both, RunSettings are different.
{ | ||
this.testAssemblies = new List<string> | ||
{ | ||
this.GetAssetFullPath("SimpleTestProject.dll"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are running acceptance test only for net451
on CI, We miss the coverage for netcore
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
this.GetAssetFullPath("SimpleTestProject2.dll") | ||
}; | ||
|
||
this.vstestConsoleWrapper = this.GetVsTestConsoleWrapper(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it good idea to create one vstestConsoleWrapper
for entire session, to mimic test window?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For maintainability, we are not doing this. In future, using Parallel may cause issues.
@@ -61,6 +61,7 @@ public void CodedWebTestRunAllTests(RunnerInfo runnerInfo) | |||
|
|||
[CustomDataTestMethod] | |||
[NETFullTargetFramework(inIsolation: true, inProcess: true)] | |||
[NETCORETargetFramework] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo); | ||
this.ExecuteNotSupportedRunnerFrameworkTests(runnerInfo.RunnerFramework, Netcoreapp, Message); | ||
|
||
string runSettingsXml = @"<?xml version=""1.0"" encoding=""utf-8""?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert for batchSize?
public void DiscoverTestsUsingDiscoveryEventHandler1(RunnerInfo runnerInfo) | ||
{ | ||
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo); | ||
this.ExecuteNotSupportedRunnerFrameworkTests(runnerInfo.RunnerFramework, Netcoreapp, Message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incase of not supported runner,skip the test.
public void RunTestsWithFastFilter(RunnerInfo runnerInfo) | ||
{ | ||
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo); | ||
this.ExecuteNotSupportedRunnerFrameworkTests(runnerInfo.RunnerFramework, Netcoreapp, Message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: rename ExecuteNotSupportedRunnerFrameworkTests
to IsSupportedRunner
.
this.vstestConsoleWrapper.RunTests( | ||
sources, | ||
this.GetDefaultRunSettings(), | ||
new TestPlatformOptions() { TestCaseFilter = "FullyQualifiedName=NUnitTestProject.NUnitTest1.PassTestMethod1" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run Failed test too.
this.runEventHandler); | ||
|
||
// Assert | ||
Assert.AreEqual(1, this.runEventHandler.TestResults.Count); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for testhost.exe.
|
||
var sources = new List<string> | ||
{ | ||
this.testEnvironment.GetTestAsset("SimpleTestProject.dll", "netcoreapp1.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explicit targetframework not required.
{ | ||
// On release, x64 builds, recursive calls may be replaced with loops (tail call optimization) | ||
Assert.Inconclusive("On StackOverflowException testhost not exited in release configuration."); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return not required.
Refer : #1239