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

Support multiple paths in TestAdapterPath argument (#1319) #1320

Merged
merged 2 commits into from
Dec 6, 2017
Merged

Support multiple paths in TestAdapterPath argument (#1319) #1320

merged 2 commits into from
Dec 6, 2017

Conversation

dtretyakov
Copy link
Contributor

Description

Currently when using VSTestTestAdapterPath as a command line parameter in msbuild or dotnet test VSTestTask bypasses it in vstest console which throws exception like that:

specified in the 'TestAdapterPath' is invalid. Error: The given path's format is not supported.

This change handles this case and merges provided paths with paths retrieved from test settings file.

Related issue

#1319, #141

@dtretyakov dtretyakov mentioned this pull request Dec 5, 2017
Closed
@smadala
Copy link
Contributor

smadala commented Dec 6, 2017

@dtretyakov Thanks for contribution 👍 .

smadala
smadala approved these changes Dec 6, 2017
View reviewed changes
Copy link
Contributor

@smadala smadala left a comment

Choose a reason for hiding this comment

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

LGTM 👍

src/vstest.console/Processors/TestAdapterPathArgumentProcessor.cs Outdated

foreach (var testadapterPath in testAdapterPaths)
{
var testAdapterFullPath = Path.GetFullPath(testadapterPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

As environment variables (%temp%\adapters) supported in runsettings file's "TestAdaptersPaths". Consider adding "Environment.ExpandEnvironmentVariables" here and test for same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smadala, ok, it was fixed.

@dtretyakov
Copy link
Contributor Author

Builds on windows machines were failed since https://dotnetcli.azureedge.net/dotnet/Sdk/2.1.0-preview1-007372/dotnet-dev-win-x64.2.1.0-preview1-007372.zip was not found. Please check this job.

@smadala
Copy link
Contributor

smadala commented Dec 6, 2017

@dotnet-bot Please test Windows_NT / Debug Build.

@smadala
Copy link
Contributor

smadala commented Dec 6, 2017

@dtretyakov Looks like issue with downloading dotnet sdk, I have trigger it one more time to check whether it was network issue or it got deleted.

@smadala
Copy link
Contributor

smadala commented Dec 6, 2017

@dotnet-bot Please test Windows_NT / Release Build.

1 similar comment
@smadala
Copy link
Contributor

smadala commented Dec 6, 2017

@dotnet-bot Please test Windows_NT / Release Build.

@singhsarab singhsarab merged commit ec73594 into microsoft:master Dec 6, 2017
@smadala smadala mentioned this pull request Dec 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mayankbansal018 mayankbansal018 mayankbansal018 approved these changes

@smadala smadala smadala approved these changes

@codito codito Awaiting requested review from codito

@pvlakshm pvlakshm Awaiting requested review from pvlakshm

@singhsarab singhsarab Awaiting requested review from singhsarab

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dtretyakov @smadala @mayankbansal018 @singhsarab