vstest icon indicating copy to clipboard operation
vstest copied to clipboard

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

Open DavidVilleneuveAnsys opened this issue 1 year ago • 7 comments

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?

DavidVilleneuveAnsys avatar Jan 29 '25 10:01 DavidVilleneuveAnsys

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.

etiennearnal avatar Jan 29 '25 10:01 etiennearnal

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

nohwnd avatar Jan 29 '25 10:01 nohwnd

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

DavidVilleneuveAnsys avatar Jan 29 '25 10:01 DavidVilleneuveAnsys

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.

nohwnd avatar Jan 29 '25 11:01 nohwnd

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

etiennearnal avatar Jan 29 '25 13:01 etiennearnal

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,

DavidVilleneuveAnsys avatar Feb 03 '25 21:02 DavidVilleneuveAnsys

@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.

nohwnd avatar Feb 04 '25 08:02 nohwnd