No support for Windows paths

[ssallmen]

I guess the library does not currently support Windows paths with backslashes. At least I did not find any way to merge for example files in a subfolder in Windows.

For example command jrm combined.xml "junit\junit_robot.xml" produces the following output even when the file is there:

PS C:\> jrm combined.xml "junit\junit_robot.xml"
Done. 0 files processed.
Provided input file patterns did not matched any file.

PS C:\> dir junit


    Directory: C:\junit


Mode                LastWriteTime         Length Name
----                -------------         ------ ----
-a----         6.9.2021     12.04           1220 junit_robot.xml
-a----         6.9.2021     15.12           4936 xjunit_jest.xml

The same issue is also present when the API (mergeFiles) is used. Tried also for example with .\junit_robot.xml when the file is in the current working directory but with the same result. Only for example junit_robot.xml or junit_*.xml (without any path separators) works.

[ssallmen]

I cheked the code and noticed that it is using the fast-glob module, which does not support windows paths, only forward slashes as path separators. I quickly tried with the following simple added code in the beginning of mergeFiles function:

const path = require('path')
module.exports.mergeFiles = function (destFilePath, srcFilePathsOrGlobPatterns, options, cb) {
  const { callback, normalizedOptions, returnValue } = normalizeArgs(options, cb)
  srcFilePathsOrGlobPatterns.forEach(function(srcFilePathOrGlobPattern, index) {
    this[index] = path.normalize(srcFilePathOrGlobPattern).replace('\\', '/')
  }, srcFilePathsOrGlobPatterns)

I don't know if there are some use cases where that solution breaks things, but at least for my cases it seems to do the trick. What it may break are the cases where the glob pattern includes double backslashes, but are there really such cases?
And yes, I am here directly modifying the function argument variable value, which should not be done, but this was just a quick trial. The final solution should not do that.

[bhovhannes]

Hi @ssallmen ,

Strictly speaking, this is not something which should be fixed in this repo. In version 3.0.0 fast-glob changed its behavior to allow only forward slashes in globs, which I think is correct, as glob originated from Unix where forward slashes are used as path separators.
There is even a related issue raised in fast-glob repo.

So, a glob junit\junit_robot.xml should not match the file junit_robot.xml, as \ just escapes the letter j which comes after. As a result this glob matches the file named unitjunit_robot.xml. The correct glob pattern is junit/junit_robot.xml which should match files in both Windows and Unix-based systems. If that is not the case, that is a bug (I don't have Windows to check that).

I understand that replacing \ with / works in your case, but I am not sure how to support Windows-style separators in glob patterns here without breaking other valid glob patterns where \ is used for escaping and not as a path separator. If you have any thoughts I am all ears.

[ssallmen]

I do understand that a backslash is escape, and that's why I said that it will cause issues in glob patterns using backslashes. And yes, the forward slashes do work in Windows, but the issue here is that it is quite incovenient for Windows users to use forward slashes in file paths or patterns since all autocompletes and copy&pastes from any place will give you backslashes as path separators. So, a Windows user will always need to replace the separators with forward slashes. But, if you are not willing to add support for Windows paths in the module, I will need to do that in my app utilizing the module.
So, I keep this more as a usability issue. Not so much as a functional issue. And from all that you may already understand that I am mainly a test engineer caring also about usability, not a developer caring only about functionality and ease of implementation.

[bhovhannes]

I've looked more into how other folks are solving this issue. It is not just a simple string replace.

Looking into slash code there are some nuances which one should take into account. Simple string replace does not work in all cases.

Also, in case of Win32 namespaces a ? character denotes a namespace and not a glob special character, so it should be escaped after converting \ to /.

I am leaning towards researching all these edge cases and make junit-report-merger to work with Windows-style glob patterns out of the box. But that will take some time.

@ssallmen can you please tell about your use case?
Do you run junit-report-merger on Windows on CI, or you run junit-report-merger in a desktop app which runs on a Windows machine, or you just run junit-report-merger by hand, or something else?

[ssallmen]

It would be really great if you could figure out how this could be implemented in a decent way, so that it works well both in Unix based systems, as well as in Windows.
My use case is such that junit-report-merger is used in a command line application that users run on Windows desktop. The app itself then sends the merged jUnit XML-file to a server (made by me) via a REST API, together with some other information. The users are not usually very used to use any Unix shells, and that way the paths used in those systems are not familiar for them. They probably won't be using very advanced glob patterns, possibly just some asterisks replacing part of the base file name. That's why the very trivial and easy solution will work for them, but possibly not for advanced users, who could use also more advanced glob patterns.

[ssallmen]

Just FYI

This is how I resolved that Windows usability issue in my application:

const os = require('os');
if (os.type().startsWith('Windows')) {
  // 'paths' is a list of file paths or glob patterns given by the user on commandline. Pipe char need to be used as escape in the patterns in Windows
  paths.forEach(function(srcFilePathOrGlobPattern, index) {
    this[index] = path.normalize(srcFilePathOrGlobPattern).replace(/\\/g, '/').replace(/\|/g, '\\');
  }, paths);
}

I replaced the backslash as escape to be pipe, or vertical bar (|), character in case of Windows. In the help of the app, I will guide Windows users to use pipe as an escape character in glob patterns, instead of backslash. I selected pipe because it is not allowed in file names in Windows.

This may not be sufficient solution for all the possible cases but at least it serves most real use cases, and people can use normal Windows paths. It may be an inconvenience for people familiar with glob patterns, but it is much more rare case, and easily solvable by the already advanced user, than that normal Windows paths would not work at all.
After some testing with different glob patterns and paths, I have not yet found any case where this would not work the same way as with forward slashes as path separators in Windows. That of course does not mean there wouldn't be any bugs, as a test engineer is always a bit suspicios 😉