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

--findRelatedTests path case sensitivity on Windows #8900

Closed
mbelsky opened this issue Sep 2, 2019 · 13 comments
Closed

--findRelatedTests path case sensitivity on Windows #8900

mbelsky opened this issue Sep 2, 2019 · 13 comments

Comments

@mbelsky
Copy link
Contributor

mbelsky commented Sep 2, 2019

🐛 Bug Report

Running jest --findRelatedTests <filepath> with upper-cased part of path does not find any test.

To Reproduce

Steps to reproduce the behavior:

  1. git clone https://github.com/mbelsky/js-problems.git
  2. cd js-problems
  3. npm i
  4. npx jest --findRelatedTests ./PROBLEMS/001-SUM.JS

Result:

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
No files found in C:\Users\publi\js\js-problems.
Make sure Jest's configuration does not exclude this directory.
To set up Jest, make sure a package.json file exists.
Jest Documentation: facebook.github.io/jest/docs/configuration.html
Pattern: .\\PROBLEMS\\001-SUM.JS - 0 matches

Expected behavior

 FAIL  problems/__tests__/001.js
  × Тестирование задачи "001-sum" (5ms)

  ● Тестирование задачи "001-sum"

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: undefined

      2 | 
      3 | test('Тестирование задачи "001-sum"', () => {
    > 4 |     expect(sum(-100)).toBe(1);
        |                       ^
      5 |     expect(sum(0)).toBe(1);
      6 |     expect(sum(1)).toBe(1);
      7 |     expect(sum(2)).toBe(3);

      at Object.toBe (problems/__tests__/001.js:4:23)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        1.228s, estimated 2s
Ran all test suites related to files matching /.\\PROBLEMS\\001-SUM.JS/i.

envinfo

  System:
    OS: Windows 10
    CPU: (4) x64 Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz
  Binaries:
    Node: 10.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.16.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
@mbelsky
Copy link
Contributor Author

mbelsky commented Sep 2, 2019

I may try to fix that issue. Could someone give me an idea from where to start, please?

@SimenB
Copy link
Member

SimenB commented Sep 2, 2019

The OP should include a reproduction, btw.


Awesome you can work on this! As for where to start, I'd start by just sticking in breakpoints SearchSource: https://github.com/facebook/jest/blob/554573036ae7529425aa157a1fd757612289abb5/packages/jest-core/src/SearchSource.ts#L280-L311

My guess is that tests are missing there, so just walk up the callstack until you find where we compare against files found on the FS. The issue is probably somewhere in https://github.com/facebook/jest/blob/554573036ae7529425aa157a1fd757612289abb5/packages/jest-resolve-dependencies/src/index.ts#L73-L136

@mbelsky
Copy link
Contributor Author

mbelsky commented Sep 4, 2019

To debug the issue I've added this config in .vscode/launch.json and set a breakpoint in packages/jest-core/src/SearchSource.ts line 268.

    {
      "type": "node",
      "request": "launch",
      "name": "Debug Jest --findRelatedTests",
      "program": "${workspaceFolder}/packages/jest-cli/bin/jest.js",
      "args": [
        "--runInBand",
        "--findRelatedTests",
        "packages/jest-core/src/FailedTestsCache.ts"
      ]
    }

https://github.com/facebook/jest/blob/554573036ae7529425aa157a1fd757612289abb5/packages/jest-core/src/SearchSource.ts#L268
And on the line I see that globalConfig.findRelatedTests and paths are undefined.

Does it mean that I use wrong launch config to debug that or globalConfig.findRelatedTests doesn't correlate with --findRelatedTests argument?

@SimenB
Copy link
Member

SimenB commented Sep 4, 2019

You're probably debugging the test and not jest itself when it executes the test. You need to put a breakpoint in the compiled code, so in packages/jest-core/build/SearchSource.js

@mbelsky
Copy link
Contributor Author

mbelsky commented Sep 8, 2019

The first issue that I face is HasteFs.exists is case-sensitive. It doesn't make real check with filesystem and just look for a key in Map. So dependencies resolving ends here for upper-cased path.
https://github.com/facebook/jest/blob/554573036ae7529425aa157a1fd757612289abb5/packages/jest-resolve-dependencies/src/index.ts#L118

I've passed this conclusion with some hacks and found one more checking in collectModules like previous.
https://github.com/facebook/jest/blob/554573036ae7529425aa157a1fd757612289abb5/packages/jest-resolve-dependencies/src/index.ts#L94

This time it's Set instead of Map and it's same issue.

I'm not sure what a solution will be more elegant to handle case insensitivity here. Do you have any suggestions?

--
btw a repro has been added.

@SimenB
Copy link
Member

SimenB commented Sep 8, 2019

Is there some check we can make for "is the FS case sensitive"? If we can, we could just normalize file paths in the Maps and Sets when the FS is case insensitive using toLowerCase() or something. I guess we could check if os.platform() === 'win32', but that seems somewhat fragile.


And issue with fixing this btw is that you might end up with scripts that work on windows but not on mac/linux (e.g. jest integrationTests), while currently all OSes behaves the same, which might be considered a feature.

@mbelsky
Copy link
Contributor Author

mbelsky commented Sep 9, 2019

What do you think about an alternative compact solution where we normalize paths with a lib like true-case-path or node-glob to restore path case for win32 platforms?
https://github.com/facebook/jest/blob/554573036ae7529425aa157a1fd757612289abb5/packages/jest-core/src/SearchSource.ts#L255

@netcoding87
Copy link

Any updates on this?

@mbelsky
Copy link
Contributor Author

mbelsky commented Dec 6, 2019

@netcoding87 I've made a pr, but some changes required. I'll try to fix them this year 😓

@netcoding87
Copy link

👍 Many thanks for your support!

@mbelsky
Copy link
Contributor Author

mbelsky commented Jan 22, 2020

Fix for this issue has been released in v25.1.0

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants