-
-
Notifications
You must be signed in to change notification settings - Fork 740
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
GH-3127: Add ResultsDirectory to VSTestSettings. (see #3127) #3128
Conversation
@devlead Please review it :) thanks |
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.
Thanks @soroshsabz. LGTM.
Could you please squash the commits and rebase against latest develop
?
@augustoproiete I done 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.
LGTM
@@ -117,6 +117,11 @@ private ProcessArgumentBuilder GetArguments(IEnumerable<FilePath> assemblyPaths, | |||
builder.AppendSwitchQuoted("/Diag", ":", settings.Diag.MakeAbsolute(_environment).FullPath); | |||
} | |||
|
|||
if (settings.ResultsDirectory != null) | |||
{ | |||
builder.AppendSwitchQuoted("/ResultsDirectory", ":", settings.ResultsDirectory.MakeAbsolute(_environment).FullPath); |
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.
👍 Looks good / prior art
/// Test results directory will be created in specified path if not exists. | ||
/// VSTest.Console.exe flag <see href="https://docs.microsoft.com/en-us/visualstudio/test/vstest-console-options#ResultDirectory">/ResultsDirectory</see>. | ||
/// </summary> | ||
public DirectoryPath ResultsDirectory { get; set; } |
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.
👍 Matches command-line argument in VSTest-Console docs
@soroshsabz your changes have been merged, thanks for your contribution 👍 |
This PR resolved #3127