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

relative path for test dlls as arguments #335

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Microsoft.TestPlatform.CoreUtilities/Helpers/FileHelper.cs
Expand Up @@ -52,5 +52,17 @@ public FileAttributes GetFileAttributes(string path)
{
return new FileInfo(path).Attributes;
}

/// <inheritdoc/>
public bool IsRootedPath(string path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we consider APIs that have filesystem interactions to be part of IFileHelper, this would allow us to inject a testable implementation easily.

Path.IsRootedPath doesn't access the file system. It is string comparison I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was considering it more of a wrapper over C# file API, so it made sense to make it a part of FileHelper

{
return Path.IsPathRooted(path);
}

/// <inheritdoc/>
public string CombinePath(string source)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this API be made more intuitive? Or should we just provide an API to get current directory. Let the caller do Path.Combine.

Path.Combine again doesn't access the file system. It may not be provided via IFileHelper.

{
return Path.Combine(Directory.GetCurrentDirectory(), source);
}
}
}
Expand Up @@ -56,5 +56,19 @@ public interface IFileHelper
/// <param name="path">Full path of the file.</param>
/// <returns>Attributes of the file.</returns>
FileAttributes GetFileAttributes(string path);

/// <summary>
/// Check if file path is rooted.
/// </summary>
/// <param name="path">Full path of the file.</param>
/// <returns>Is file path rooted.</returns>
bool IsRootedPath(string path);

/// <summary>
/// Create full file path.
/// </summary>
/// <param name="path">relative path of the file.</param>
/// <returns>Full file path, based on current working directory.</returns>
string CombinePath(string source);
}
}
6 changes: 6 additions & 0 deletions src/vstest.console/CommandLine/CommandLineOptions.cs
Expand Up @@ -253,6 +253,12 @@ public void AddSource(string source)
}

source = source.Trim();

if(!FileHelper.IsRootedPath(source))
Copy link
Contributor

Choose a reason for hiding this comment

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

We need tests for this

{
source = FileHelper.CombinePath(source);
}

if (!FileHelper.Exists(source))
{
throw new CommandLineException(
Expand Down