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
9 changes: 8 additions & 1 deletion src/vstest.console/Internal/FilePatternParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,14 @@ 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 (either \ or /) before the wildcard
// On Windows, both \ and / are valid directory separators
// On Unix-like systems, only / is typically valid, but this approach is safe for both
var directorySeparatorIndex = Math.Max(
pathBeforeWildCard.LastIndexOf(Path.DirectorySeparatorChar),
pathBeforeWildCard.LastIndexOf(Path.AltDirectorySeparatorChar));
Copy link
Member

Choose a reason for hiding this comment

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

use just the appropriate separator depending on OS. On linux use just forward DirectorySeparatorChar

Copy link
Member

Choose a reason for hiding this comment

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

@copilot address this feedback if you did not already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feedback has been addressed in commit 0ab1d4b. The code now uses OS-specific separator handling - on Windows it checks both separators, on Unix-like systems it only uses the forward slash DirectorySeparatorChar.


string searchDir = filePattern.Substring(0, directorySeparatorIndex);
string pattern = filePattern.Substring(directorySeparatorIndex + 1);
Expand Down
49 changes: 49 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,55 @@ public void FilePatternParserShouldThrowCommandLineExceptionIfFileDoesNotExist()
Assert.ThrowsException<TestSourceException>(() => _filePatternParser.GetMatchingFiles(TranslatePath(@"E:\path\to\project\tests\Blame.Tests\\abc.Tests.dll")));
}

[TestMethod]
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/vanidhi/Desktop/a/c/*bc.dll");
Copy link
Member

Choose a reason for hiding this comment

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

don't use vanidhi in the examples, use someUser instead.

Copy link
Member

Choose a reason for hiding this comment

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

@copilot address this feedback if you did not already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feedback has been addressed in commit 0ab1d4b. The test now uses "someUser" instead of "vanidhi" in the example path.


// 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("vanidhi") && y.FullName.Contains("Desktop") &&
y.FullName.Contains("a") && y.FullName.EndsWith("c"))));
}

[TestMethod]
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/vanidhi/**/c/*bc.txt");

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

[TestMethod]
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
Loading