-
Notifications
You must be signed in to change notification settings - Fork 323
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
Always look for adapters in Test Source directory #1574
Always look for adapters in Test Source directory #1574
Conversation
this.testHostManager.Initialize(this.mockMessageLogger.Object, $"<?xml version=\"1.0\" encoding=\"utf-8\"?><RunSettings> <RunConfiguration><TestAdaptersPaths>C:\\Foo</TestAdaptersPaths></RunConfiguration> </RunSettings>"); | ||
List<string> currentList = new List<string> { @"FooExtension.dll" }; | ||
List<string> currentList = new List<string> { @"FooExtension.dll", @"C:\Source1\ext1.TestAdapter.dll", @"C:\Source1\ext2.TestAdapter.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.
Change test name.
@@ -192,7 +191,7 @@ public TestHostConnectionInfo GetTestHostConnectionInfo() | |||
/// <inheritdoc/> | |||
public IEnumerable<string> GetTestPlatformExtensions(IEnumerable<string> sources, IEnumerable<string> extensions) | |||
{ | |||
if (sources != null && sources.Any() && this.projectOutputExtensionsRequired) | |||
if (sources != null && sources.Any()) |
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.
Remove explicitly testadapterpath from acceptance tests. Example.
foreach (var arg in this.VSTestTestAdapterPath) | ||
{ | ||
allArgs.Add("--testAdapterPath:" + ArgumentEscaper.HandleEscapeSequenceInArgForProcessStart(arg)); | ||
} | ||
} | ||
else |
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.
We can remove else condition.
|
||
if (arg.StartsWith("console", StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
initializeConsoleLogger = 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.
Nit: Rename to isConsoleLoggerEnabled
or something close to it?
var allArguments = vstestTask.CreateArgument().ToArray(); | ||
|
||
Assert.IsNotNull(allArguments.FirstOrDefault(arg => arg.Contains("--testAdapterPath:path1"))); | ||
Assert.IsNotNull(allArguments.FirstOrDefault(arg => arg.Contains("--testAdapterPath:path1"))); |
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.
path2
.
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.
Please mention the testing done with respect to changes associated with dotnet cli change.
Approving with suggestions.
@@ -54,7 +54,7 @@ public string VSTestTestCaseFilter | |||
get; | |||
set; | |||
} | |||
public string VSTestLogger | |||
public string[] VSTestLogger |
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: Should we rename these to reflect the change, VSTestLoggers ?
vstestTask.VSTestFramework = "abc"; | ||
|
||
vstestTask.VSTestLogger = new string[] { "trx;LogFileName=foo bar.trx", "console" }; | ||
|
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: extra line
Also, better for required properties, use new VSTestTask { TestFileFullPath = "abc", VSTestFramework = "xyz"}
@@ -31,7 +31,7 @@ public string VSTestSetting | |||
set; | |||
} | |||
|
|||
public string VSTestTestAdapterPath | |||
public string[] VSTestTestAdapterPath |
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.
string[] [](start = 15, length = 8)
nitpick: IEnumerable over string[]
@dotnet-bot test Windows_NT / Debug Build please |
1 similar comment
@dotnet-bot test Windows_NT / Debug Build please |
Description
Dotnet CLI PR: dotnet/cli#9198
Related issue
Fixes #1319