Fix absolute path check for Windows

[brandonchinn178]

Summary

Fixes #15109, regression introduced in #12519

Test plan

[netlify]

✅ Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	3835811
🔍 Latest deploy log	https://app.netlify.com/sites/jestjs/deploys/66b2e56d18981e0008ec348f
😎 Deploy Preview	https://deploy-preview-15235--jestjs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[SimenB]

Right, this seems reasonable. Can we add a test as well?

[connectdotz]

wouldn't the following block, where it checks for a relative path, suffer the same problem for windows relative paths?

[brandonchinn178]

@connectdotz You mean because it would be .\ instead of ./?

[connectdotz]

@brandonchinn178

You mean because it would be .\ instead of ./?

Yes.

I saw your change, and it should work for most cases. However, I did notice at least one use case where it might not work: a relative path pattern with a leading escape, such as "\\.\\\\subdir\\\\file\\.test\\.ts" (regex for .\subdir\file.test.ts). I think even path.isAbsolute() might fail on this one (it would return true instead of false).

It seems the complexity here is due to the fact that we are dealing with a regex pattern for the path, not the path itself.

In any case, our extension doesn't use relative path patterns, so this may not be relevant for us. However, I wanted to bring this potential issue to your attention so you can decide how (or if) you'd like to handle it.

[brandonchinn178]

I don't think that's an expected usage. We use a regex to match, yes, but the user input isnt a regex; in isMatch, we escape the entire path to make it a literal regex.

I think I tried doing something originally like "strip root directory from the test filename, then check if filter is a substring in test filename" and not use regex at all, but I think it broke something else...

[connectdotz]

We use a regex to match, yes, but the user input isn't a regex

Doesn't --testPathPatterns take regex input strings?

However, I do agree that to match a file like c:\whatever\subdir\file.test.ts with a partial regex pattern, one should use "\\\\subdir\\\\file\\.test\\.ts", without the leading ".", with or without the escape. In other words, I'm not sure we need to handle the .${path.sep} pattern at all here; the match should fail if people pass in such a regex string.

That said, we should probably model the implementation using the current jest --testPathPattern as guidance to ensure it is as backward compatible as possible.

[brandonchinn178]

@connectdotz You're right, it is supposed to be a regex. This is what I get for skimming PRs on my phone 😄

However, your example here:

a relative path pattern with a leading escape, such as "\.\\subdir\\file\.test\.ts" (regex for .\subdir\file.test.ts)

I don't think this ever worked. I just tested this in a new project with jest 29. The reason that ./foo.spec.js worked is IMO a bit of an accident. The old jest behavior just converted it into a regex verbatim (/.\/foo.spec.js/i) and then matched it against the full path (/home/jdoe/my-project/foo.spec.js) and the ./ matched the end of projec(t/).

@SimenB You're right, I just took another look at the docs.

I think the problem here is that this option tries to be both a path and a regex. This has some odd behavior:

$ cd /home/jdoe/my-project/
$ tree .
.
|-- foo.spec.js
|-- dir1/
    |-- dir2/
        |-- bar.spec.js

$ jest './foo.spec.js'
Path: /home/jdoe/my-project/foo.spec.js           => MATCH (t/foo.spec.js)
Path: /home/jdoe/my-project/dir1/dir2/bar.spec.js => NO MATCH

$ jest '\./foo.spec.js'
Path: /home/jdoe/my-project/foo.spec.js           => NO MATCH
Path: /home/jdoe/my-project/dir1/dir2/bar.spec.js => NO MATCH

$ jest '../foo.spec.js'
Path: /home/jdoe/my-project/foo.spec.js           => MATCH (ct/foo.spec.js)
Path: /home/jdoe/my-project/dir1/dir2/bar.spec.js => NO MATCH

$ jest 'dir1/../foo.spec.js'
Path: /home/jdoe/my-project/foo.spec.js           => NO MATCH
Path: /home/jdoe/my-project/dir1/dir2/bar.spec.js => NO MATCH

$ jest './bar.spec.js'
Path: /home/jdoe/my-project/foo.spec.js           => NO MATCH
Path: /home/jdoe/my-project/dir1/dir2/bar.spec.js => MATCH (2/bar.spec.js)

@SimenB Can I propose we have the following semantics:

1. If the path is absolute, match it as a regex to the full test path
2. Otherwise, strip the rootDir from the test path and match the pattern to the rest

This most closely matches the spirit of the option, it seems like. The only breaking change here is that ./foo.spec.js will no longer work, but that seems more logically consistent with the fact that a period everywhere else is a regex dot, not a directory dot.

The alternative is to special case the leading ./ on POSIX (and both ./ and .\ on WINDOWS), but I'm not sure what the best "special case" behavior is here:

1. Should a leading ./ just be stripped? So ./foo.spec.js would match any foo.spec.js
2. Should it force resolution at rootDir? This is a breaking change where ./bar.spec.js would no longer work

[SimenB]

I would have preferred them being globs rather than regexes, but I guess it's way to late to change that now 😅

jest ./some-local.test.js needs to work as that's a very natural way to define a file path. So I guess your alternative, with 2 is the way to go?

2. This is a breaking change where ./bar.spec.js would no longer work

Seems fine - you're passing a relative path, it should probably only match relatively to where the command is used?

[SimenB]

Latest changes look promising! @connectdotz could you give them a quick test?

[connectdotz]

@connectdotz could you give them a quick test?

Yes, it works with our use case. 👍

[SimenB]

thanks!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.