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.

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

Added an option to deploy all files in the test source location #391

Merged
merged 9 commits into from
Oct 31, 2018
Merged

Added an option to deploy all files in the test source location #391

merged 9 commits into from
Oct 31, 2018

Conversation

cltshivash
Copy link
Contributor

@cltshivash cltshivash commented Mar 27, 2018

Added an option to deploy the test source dependencies and satellite dlls by default (DeployTestSourceDependencies) in addition to the content of the test source directory (bin/debug of the test dll). This option will be on by default for compat reasons.

It can be turned off in which case deployment would only deploy the files from the test source location.

Added UTs and E2E tests for the change

…dlls by default. This option will be on by default.

It can turned off in which case deployment would just deploy the test source location and skip the test results folder if the test results folder is within it.

Added UTs and E2E tests for the change
@smadala smadala removed their assignment May 8, 2018
@singhsarab
Copy link
Contributor

            RunDirectories = runDirectories;

This updation is not clear. Do we still need this?


Refers to: src/Adapter/PlatformServices.Desktop/Services/DesktopTestDeployment.cs:158 in a43a940. [](commit_id = a43a940, deletion_comment = False)

Copy link
Contributor

@singhsarab singhsarab left a comment

Choose a reason for hiding this comment

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

Approving with few suggestions

@@ -116,6 +57,7 @@ public static MSTestAdapterSettings ToSettings(XmlReader reader)
//
// <MSTestV2>
// <DeploymentEnabled>true</DeploymentEnabled>
// <DeployTestSourceDependencies>true</DeployTestSourceDependencies>
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

@@ -91,14 +92,14 @@ internal bool IsValidDeploymentItem(string sourcePath, string relativeOutputDire
return false;
}

if (sourcePath.IndexOfAny(System.IO.Path.GetInvalidPathChars()) != -1 ||
relativeOutputDirectory.IndexOfAny(System.IO.Path.GetInvalidPathChars()) != -1)
if (sourcePath.IndexOfAny(Path.GetInvalidPathChars()) != -1 ||
Copy link
Member

Choose a reason for hiding this comment

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

👍

using System.Threading.Tasks;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Deployment;

internal class DeploymentResult
Copy link
Member

Choose a reason for hiding this comment

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

Adding comments summarizing purpose of this class would be good.

@jayaranigarg
Copy link
Member

jayaranigarg commented May 11, 2018

Please put up an RFC in testfx-docs describing the usage of this 'DeployTestSourceDependencies' flag and link that to this PR.

@jayaranigarg
Copy link
Member

@cltshivash : Please complete this PR.

@jayaranigarg
Copy link
Member

@cltshivash : Can we get this PR in? It will solve this issue #490 as well.

@mayankbansal018 mayankbansal018 merged commit e0fb2a8 into microsoft:master Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants