Skip to content

[WIP] using globbing pattern doesn't work on windows with forward slashes #15088

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft
30 changes: 30 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
This is a .NET based repository that contains the VSTest test platform. Please follow these guidelines when contributing:

## Code Standards

You MUST follow all code-formatting and naming conventions defined in [`.editorconfig`](../.editorconfig).

In addition to the rules enforced by `.editorconfig`, you SHOULD:

- Favor style and conventions that are consistent with the existing codebase.
- Prefer file-scoped namespace declarations and single-line using directives.
- Ensure that the final return statement of a method is on its own line.
- Use pattern matching and switch expressions wherever possible.
- Use `nameof` instead of string literals when referring to member names.
- Always use `is null` or `is not null` instead of `== null` or `!= null`.
- Trust the C# null annotations and don't add null checks when the type system says a value cannot be null.
- Prefer `?.` if applicable (e.g. `scope?.Dispose()`).
- Use `ObjectDisposedException.ThrowIf` where applicable.
- Respect StyleCop.Analyzers rules, in particular:
- SA1028: Code must not contain trailing whitespace
- SA1316: Tuple element names should use correct casing
- SA1518: File is required to end with a single newline character

You MUST minimize adding public API surface area but any newly added public API MUST be declared in the related `PublicAPI.Unshipped.txt` file.

## Localization Guidelines

Anytime you add a new localization resource, you MUST:
- Add a corresponding entry in the localization resource file.
- Add an entry in all `*.xlf` files related to the modified `.resx` file.
- Do not modify existing entries in '*.xlf' files unless you are also modifying the corresponding `.resx` file.
20 changes: 19 additions & 1 deletion src/vstest.console/Internal/FilePatternParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Runtime.InteropServices;

using Microsoft.Extensions.FileSystemGlobbing;
using Microsoft.Extensions.FileSystemGlobbing.Abstractions;
Expand Down Expand Up @@ -96,7 +97,24 @@ private Tuple<string, string> SplitFilePatternOnWildCard(string filePattern)
{
// Split the pattern based on first wild card character found.
var splitOnWildCardIndex = filePattern.IndexOfAny(_wildCardCharacters);
var directorySeparatorIndex = filePattern.Substring(0, splitOnWildCardIndex).LastIndexOf(Path.DirectorySeparatorChar);
var pathBeforeWildCard = filePattern.Substring(0, splitOnWildCardIndex);

// Find the last directory separator before the wildcard
// On Windows, we need to check both \ and / as both are valid
// On Unix-like systems, only / is the directory separator
int directorySeparatorIndex;
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
// On Windows, check both separators and take the last one found
directorySeparatorIndex = Math.Max(
pathBeforeWildCard.LastIndexOf(Path.DirectorySeparatorChar),
pathBeforeWildCard.LastIndexOf(Path.AltDirectorySeparatorChar));
}
else
{
// On Unix-like systems, only use the forward slash
directorySeparatorIndex = pathBeforeWildCard.LastIndexOf(Path.DirectorySeparatorChar);
}

string searchDir = filePattern.Substring(0, directorySeparatorIndex);
string pattern = filePattern.Substring(directorySeparatorIndex + 1);
Expand Down
55 changes: 55 additions & 0 deletions test/vstest.console.UnitTests/Internal/FilePatternParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,61 @@ public void FilePatternParserShouldThrowCommandLineExceptionIfFileDoesNotExist()
Assert.ThrowsException<TestSourceException>(() => _filePatternParser.GetMatchingFiles(TranslatePath(@"E:\path\to\project\tests\Blame.Tests\\abc.Tests.dll")));
}

[TestMethod]
// only on windows because we don't translate the path to be valid linux / mac path
[OSCondition(OperatingSystems.Windows)]
public void FilePatternParserShouldCorrectlySplitPatternAndDirectoryWithForwardSlashes()
{
var patternMatchingResult = new PatternMatchingResult(new List<FilePatternMatch>());
_mockMatcherHelper.Setup(x => x.Execute(It.IsAny<DirectoryInfoWrapper>())).Returns(patternMatchingResult);

// Test with forward slashes - this should work on all platforms
// This specifically tests the fix for issue #14993
_filePatternParser.GetMatchingFiles("C:/Users/someUser/Desktop/a/c/*bc.dll");

// Assert that the pattern is parsed correctly
_mockMatcherHelper.Verify(x => x.AddInclude("*bc.dll"));
// On Windows, the path may be normalized, so we verify the key components are present
_mockMatcherHelper.Verify(x => x.Execute(It.Is<DirectoryInfoWrapper>(y =>
y.FullName.Contains("someUser") && y.FullName.Contains("Desktop") &&
y.FullName.Contains("a") && y.FullName.EndsWith("c"))));
}

[TestMethod]
// only on windows because we don't translate the path to be valid linux / mac path
[OSCondition(OperatingSystems.Windows)]
public void FilePatternParserShouldCorrectlySplitWithArbitraryDirectoryDepthWithForwardSlashes()
{
var patternMatchingResult = new PatternMatchingResult(new List<FilePatternMatch>());
_mockMatcherHelper.Setup(x => x.Execute(It.IsAny<DirectoryInfoWrapper>())).Returns(patternMatchingResult);

// Test with forward slashes and recursive patterns
_filePatternParser.GetMatchingFiles("C:/Users/someUser/**/c/*bc.txt");

// Assert
_mockMatcherHelper.Verify(x => x.AddInclude("**/c/*bc.txt"));
_mockMatcherHelper.Verify(x => x.Execute(It.Is<DirectoryInfoWrapper>(y =>
y.FullName.Contains("someUser"))));
}

[TestMethod]
// only on windows because we don't translate the path to be valid linux / mac path
[OSCondition(OperatingSystems.Windows)]
public void FilePatternParserShouldHandleForwardSlashesWithoutThrowingException()
{
var patternMatchingResult = new PatternMatchingResult(new List<FilePatternMatch>());
_mockMatcherHelper.Setup(x => x.Execute(It.IsAny<DirectoryInfoWrapper>())).Returns(patternMatchingResult);

// This is the specific case from the original bug report that was throwing ArgumentOutOfRangeException
// Before the fix: System.ArgumentOutOfRangeException: length ('-1') must be a non-negative value
_filePatternParser.GetMatchingFiles("C:/path/to/my/tests/*_Tests.dll");

// Assert that we successfully parse without throwing and get the expected pattern
_mockMatcherHelper.Verify(x => x.AddInclude("*_Tests.dll"));
_mockMatcherHelper.Verify(x => x.Execute(It.Is<DirectoryInfoWrapper>(y =>
y.FullName.Contains("path") && y.FullName.Contains("tests"))));
}

private static string TranslatePath(string path)
{
// RuntimeInformation has conflict when used
Expand Down