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

using globbing pattern doesn't work on windows with forward slashes #14993

Open
DavidVilleneuveAnsys opened this issue Jan 29, 2025 · 7 comments
Labels

Comments

@DavidVilleneuveAnsys
Copy link

On windows, when calling dotnet test C:/path/to/my/tests/*_Tests.dll we get the following errors :

Unhandled exception. System.ArgumentOutOfRangeException: length ('-1') must be a non-negative value. (Parameter 'length')
Actual value was -1.
   at System.ArgumentOutOfRangeException.ThrowNegative[T](T value, String paramName)
   at System.ArgumentOutOfRangeException.ThrowIfNegative[T](T value, String paramName)
   at System.String.ThrowSubstringArgumentOutOfRange(Int32 startIndex, Int32 length)
   at System.String.Substring(Int32 startIndex, Int32 length)
   at vstest.console.Internal.FilePatternParser.SplitFilePatternOnWildCard(String filePattern) in /_/src/vstest.console/Internal/FilePatternParser.cs:line 101
   at vstest.console.Internal.FilePatternParser.GetMatchingFiles(String filePattern) in /_/src/vstest.console/Internal/FilePatternParser.cs:line 75
   at Microsoft.VisualStudio.TestPlatform.CommandLine.CommandLineOptions.AddSource(String source) in /_/src/vstest.console/CommandLine/CommandLineOptions.cs:line 283
   at Microsoft.VisualStudio.TestPlatform.CommandLine.Processors.ArgumentProcessorFactory.<>c__DisplayClass18_0.<WrapLazyProcessorToInitializeOnInstantiation>b__0() in /_/src/vstest.console/Processors/Utilities/ArgumentProcessorFactory.cs:line 280
   at System.Lazy`1.CreateValue()
   at Microsoft.VisualStudio.TestPlatform.CommandLine.Executor.GetArgumentProcessors(String[] args, List`1& processors) in /_/src/vstest.console/CommandLine/Executor.cs:line 283
   at Microsoft.VisualStudio.TestPlatform.CommandLine.Executor.Execute(String[] args) in /_/src/vstest.console/CommandLine/Executor.cs:line 173
   at Microsoft.VisualStudio.TestPlatform.CommandLine.Program.Main(String[] args) in /_/src/vstest.console/Program.cs:line 22

This works when using backward slashes.

I think that since forward slashes work in general when doing other Windows CLI tools, or well, in dotnet test when not using globbing.

I feel like it could be addressed by changing the SplitFilePatternOnWildCard to take into account Path.AltDirectorySeparatorChar

https://learn.microsoft.com/en-us/dotnet/api/system.io.path.altdirectoryseparatorchar?view=net-9.0

That said I don't know how Path.AltDirectorySeparatorChar would affect other platforms?

@etiennearnal
Copy link

As far as it's supported by dotnet build C:/path/to/my/project.vcxproj, it should work with dotnet test too. Seems like a real bug.

@nohwnd nohwnd added the bug label Jan 29, 2025
@nohwnd
Copy link
Member

nohwnd commented Jan 29, 2025

Yes looks like a bug. Do you want to make a fix for it?

@DavidVilleneuveAnsys
Copy link
Author

Sure, I can attempt a fix in the week. Thanks for the prompt reply.

@nohwnd
Copy link
Member

nohwnd commented Jan 29, 2025

one thing to maybe look into:

I think we might use the globbing package in vstest somewhere, just not here.

https://www.nuget.org/packages/Microsoft.Extensions.FileSystemGlobbing

So I would look if vstest.console already references that and if it has the needed functionality.

@etiennearnal
Copy link

I reproduced with absolute path only (dotnet test C:/*_UT.dll).
No issue if I use relative glob expression (dotnet test ../*_UT.dll).

@DavidVilleneuveAnsys
Copy link
Author

I was able to reproduce the bug with an Unit test in FilePatternParserTests, by adding this test:

  [TestMethod]
  public void FilePatternParserShouldCorrectlySplitPatternAndDirectoryWithAlternateSeparator()
  {
      var patternMatchingResult = new PatternMatchingResult(new List<FilePatternMatch>());
      _mockMatcherHelper.Setup(x => x.Execute(It.IsAny<DirectoryInfoWrapper>())).Returns(patternMatchingResult);
      _filePatternParser.GetMatchingFiles(TranslatePath(@"C:/Users/vanidhi/Desktop/a/c/*bc.dll"));

      // Assert
      _mockMatcherHelper.Verify(x => x.AddInclude(TranslatePath(@"*bc.dll")));
      _mockMatcherHelper.Verify(x => x.Execute(It.Is<DirectoryInfoWrapper>(y => y.FullName.Equals(TranslatePath(@"C:\Users\vanidhi\Desktop\a\c")))));
  }

This indeed doesn't pass with current code.

For the fix, I have done the following change to the FilePatternParser class.

Added the following line where the fields are declared:

private readonly char[] _directorySeparatorCharacters = [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar];

And then modify line 100, where we search for the last directory separator to look like this:

var directorySeparatorIndex = filePattern.Substring(0, splitOnWildCardIndex).LastIndexOfAny(_directorySeparatorCharacters);

These changes fixes the previous test.

I would've pushed a branch and create a pull request, but I don't seem to have the rights to.

Thanks,

@nohwnd
Copy link
Member

nohwnd commented Feb 4, 2025

@DavidVilleneuveAnsys yes you don't have permisions to write into this repository. You need to go and fork the repository first to get your own copy where you have permissions. Then you will push the branch to your fork, and then you can create the PR. It should automatically suggest our repository in the PR user interface.

Let me know if you need more help making the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants