jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.05k stars 6.44k forks source link

Import should be case sensitive #10398

Open ya3ya6 opened 4 years ago

ya3ya6 commented 4 years ago

🐛 Bug Report :

let's say we have a module named: Autosuggest. and we want to mock it.

import AutoSuggest from './AutoSuggest' // the filename is Autosuggest.js (lower s)
jest.mock('./AutoSuggest', () => ({get : jest.fn(() => 1000)}));

if we import it with camel cases, like: AutoSuggest, it doesn't throw any error. it just let us to do it, and it allow us to mock it. but the mocking actually doesn't work this way.

Why it's important to fix : After one day of debugging about why mocking doesn't work, i fixed it with fixing the cases. so the problem is : it increases debug time.

Expected bahaviur :

jest.mock('./AutoSuggest') should throw an error when there is a module called ./Autosuggest, but there isn't any module called ./AutoSuggest.

SimenB commented 4 years ago

Agreed. I think it works on mac and windows since the FS is case insensitive but breaks correctly on linux. I haven't verified, tho!

This should be fixed somewhere in jest-resolve

SimenB commented 4 years ago

I'm on a mac and I don't get an error for case errors

ya3ya6 commented 4 years ago

I'm on windows . so it doesn't throw error (the bug exists) on mac and windows. not sure about linux. ( Just searched it and it seems some mac versions are case-sensitive, some are case-insensitive: https://qr.ae/pN2IZB )

lh0x00 commented 3 years ago

I've seen this before... In Window and MacOS are case-insensitive and Linux is case-sensitive.

sarathps93 commented 3 years ago

@SimenB Can I work on this issue?

anje123 commented 3 years ago

@SimenB is this issue avaliable can i give it a shot

SimenB commented 3 years ago

Sure! All help is very much welcome 🙂

anje123 commented 3 years ago

@SimenB I noticed the ModuleMockerClass in the jest-mock package handles mocking but how can i access the module-path jest.mock(module-path) in the class. If I am getting it wrong please guide me through

SimenB commented 3 years ago

You shouldn't have to change anything in jest-mock, I think you'll want to change stuff in https://github.com/facebook/jest/blob/master/packages/jest-resolve/src/index.ts

anje123 commented 3 years ago

@SimenB after so much trial , i could not find a way to solve this issue, tried some technique and also tried installing some packages, still didn't work , i think this is not a jest issue but an issue with macOS and window due to its case insensitivity, but i'm still currently open to any suggestion

xavierstampslafont commented 3 years ago

Hi, I'd like to look into this too :)

xavierstampslafont commented 3 years ago

I'm looking at jest-resolve's resolveModuleFromDirIfExists (https://github.com/facebook/jest/blob/master/packages/jest-resolve/src/index.ts#L136), which is called externally in the runtime's _requireResolve (https://github.com/facebook/jest/blob/master/packages/jest-resolve/src/index.ts#L136).

That looks to me like the spot that resolves the path for any given module. Internally in jest-resolve, most functions talk about resolving Node modules... or is it a "node" in the more general sense? 🤔

jest-resolve's resolve.test.ts doesn't have any tests covering resolveModuleFromDirIfExists so OS case sensitivity isn't tested. I think that resolveModuleFromDirIfExists is the culprit so going to try to fix it and add appropriate tests.

xavierstampslafont commented 3 years ago

@ya3ya6 how did Jest behave when you had the incorrect casing? Unfortunately neither of the 2 machines available to me are case-sensitive

xavierstampslafont commented 3 years ago

After I made my PR, I realized that while it helps out people on case-insensitive machines, it doesn't address anything for people on case-sensitive machines. So I'm looking at it again.

I've been trying to mock graceful-fs so that I can mock returns from each type of file system, but the mocking isn't working. I tried to set it up like the test for nodeModulesPaths (https://github.com/facebook/jest/blob/master/packages/jest-resolve/src/__tests__/resolve.test.ts#L273) but I just found out that one doesn't work either, it still loads the real graceful-fs, so maybe it hasn't worked for a while and has been giving false positives in the tests?

I also tried mocking tryRealPathtryRealpath itself, but no luck there either. I set it up the same way as the watch tests (https://github.com/facebook/jest/blob/master/packages/jest-core/src/__tests__/watch.test.js#L115).

Any suggestions at this point are welcome. As long as I can get to mock the behavior of both case-sensitive and -insensitive file systems, the bug shouldn't be too hard to fix.

@SimenB do you have any ideas? Thank you!

xavierstampslafont commented 3 years ago

Set up a Linux VM and tested the case of trying to mock a module but with incorrect casing, so that I could see how Jest behaved. It throws the Cannot find module '${moduleName}' from '${relativePath}' error.

I've made some changes in my local branch so that in this scenario, it still throws this error but also appends a list of similarly named modules. In the OP's case, it would look like this:

Cannot find module 'AutoSuggest' from 'src'. Did you mean to import one of: Autosuggest.js?

Feel free to suggest a better messaging :)

I still don't have a way of unit testing it though. Ideas on how to do so are welcome!

EDIT: lol... suddenly remembered that you can mock anything and everything. Wrote some tests that simulate the behavior of case-sensitive and case-insensitive file systems and pushed my changes.

xavierstampslafont commented 3 years ago

I think the PR is in a good state now (https://github.com/facebook/jest/pull/10794), and the tests are all passing :)

Summarizing what I've implemented in the PR, here is what you see on a case-sensitive file system: image

And on a case-insensitive file system: image

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

spawnia commented 2 years ago

We have hit this issue in one of our projects. We had files called Priority.tsx and priority.ts in the same directory, that lead to Jest crashing with a cryptic error. Renaming one of the files did the trick.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.