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 <spaceSeparatedListOfSourceFiles> don't find test file if path include special characters and file extension #11534

Closed
blephy opened this issue Jun 5, 2021 · 19 comments · Fixed by #11548

Comments

@blephy
Copy link

blephy commented Jun 5, 2021

🐛 Bug Report

Hi guys,

Implementing Stryker on my repository was a really difficult task and after searching why jest give me a lot of additional work, i'm openning this issue.

I think the code responsibly to parse and find test paths with the --findRelatedTests contains (maybe, i'm not sure) a bug.
Every things is explained in this other issue but to summarize, in my context i have an angular project which is running with jest.
Architecture folders are like this :

├── src (application codebase)
│   ├── app
│   │   ├── @core (local libraries)
│   │   ├── @shared (shared modules)
│   │   ├── app
│   │   ├── ** (main views / containers)

if i run npx jest --findRelatedTests src/app/app.module.ts everything is ok. Jest found 2 spec files. (except 240 seconds is needed to pass them ... for just 2 assertions - codebase = 29K pure ligne of code)

if i run npx jest --findRelatedTests src/app/@core/store/store.ts jest stdout this :

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In C:\Users\xxxxxx\Projects\xxxxxxxxxxxxxxxxxxxxx
  977 files checked.
  testMatch: **/__tests__/**/*.[jt]s?(x), **/?(*.)+(spec|test).[tj]s?(x) - 333 matches
  testPathIgnorePatterns: \\node_modules\\ - 977 matches
  testRegex:  - 0 matches
Pattern: src\\app\\@core\\store\\store.ts - 0 matches

To Reproduce

git clone https://github.com/blephy/jest-path-bug.git
cd jest-path-bug
npm i
npm run test:every

# look at the package.json to see paths

Expected behavior

Jest should understand paths with special characters and with or without file extensions

envinfo

├── @stryker-mutator/[email protected]
├── @stryker-mutator/[email protected]
├── @stryker-mutator/[email protected]
├── [email protected]
├── @angular/[email protected]
├── @angular-builders/[email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]

Regards !

and many thanks about maintaining jest ;)

@ahnpnl
Copy link
Contributor

ahnpnl commented Jun 6, 2021

paths is a tsconfig option which Jest doesn’t know about because internally Jest doesn’t read user tsconfig which explains why Jest cannot find any test files based on the path with @ (I assume you defined in your tsconfig)

@blephy
Copy link
Author

blephy commented Jun 6, 2021

Ok so if we set absolute path in tsconfig like below, we cannot use --findRelatedTests options ?

"paths": {
    "@core/*": ["./src/app/@core/*"],
    "@shared/*": ["./src/app/@shared/*"],
    "@environments/*": ["./src/environments/*"]
}

I'm perplexed by your answer. my src/app/@core/store/store.ts file exist, the folder @core has this name in my hard drive. This is not a computed path based on tsconfig paths option. :/

@ahnpnl
Copy link
Contributor

ahnpnl commented Jun 6, 2021

  • --findRelatedTests doesn't work with tsconfig paths .
  • if the folder name does contain @core but Jest still cannot find any related tests, it can be a bug for Jest.

@blephy
Copy link
Author

blephy commented Jun 6, 2021

That's exactly what i mean.

  • --findRelatedTests doesn't work with tsconfig paths .

I'm not using tsconfig path with jest --findRelatedTests. I'm using reel hard drive paths.

  • if the folder name does contain @core but Jest still cannot find any related tests, it can be a bug for Jest.

So i think there is definitly a bug.

My architecture folder are (i mean, on the hard drive, not computed by any tsconfig / typescript paths option) like this :

├── src (application codebase)
│   ├── app
│   │   ├── @core (local libraries)
│   │   ├── @shared (shared modules)
│   │   ├── styles (shared styles)
│   │   └── ... (main modules and views)

I'm using paths tsconfig options only in files to be able to import some files based on absolute path instead of long relative paths like ../../../../../../../@core/.... But in fact this is not the point, because my app is transpiled correctly before jest treat it.

What i mean is when i'm asking jest with npx jest --findRelatedTests src/app/@core/store/store.ts jest tells me that they are no files based on this path, which is absolutely wrong.

And why i'm supposing a special characters bug, this is because npx jest --findRelatedTests src/app/app.module.ts works. Any commands including a @ in path in my case doesn't work.

@blephy
Copy link
Author

blephy commented Jun 6, 2021

Ok i worked a bit more to show you. This is a really strange behavior.

git clone https://github.com/blephy/jest-path-bug.git
cd jest-path-bug
npm i
npm run test:every

# look at the package.json to see paths

I don't know what to think about. The bug appears more complicated as expected.

  1. If path includes a special characters, with file extension, it breaks
  2. If path includes a special characters, without file extension, that's ok !
  3. With or without file extension on some special characters in a folder, it breaks
  4. With or without file extension and without special characters, that's ok.

If it's a desired behavior, the behavior is painfull. I prefer to believe it's a bug.

In my opinion, we need some investigations

EDIT: i saw a lot of clone, please git pull because i updated npm scripts tests to reflect more cases

@blephy blephy changed the title --findRelatedTests <spaceSeparatedListOfSourceFiles> don't find anything with special characters in path --findRelatedTests <spaceSeparatedListOfSourceFiles> don't find test file if path include special characters and file extension Jun 6, 2021
@blephy
Copy link
Author

blephy commented Jun 6, 2021

What about this contract (by order of preference) :

  • If path doesn't include extension file
    • jest should check if path is an existing folder
      • if path is a folder, jest should run all test files in the folder
      • if there is no folder, jest should try to find a related spec file by adding all common extension to the given name (.js, .jsx, .ts, .tsx)
      • otherwise, an stdout with no spec files found should be return.
  • if path include extension file
    • jest should try to find a related spec file with the extension given
    • otherwise, an stdout with no spec files found should be return.
  • Special characters must always be taken into consideration. It's not great to impose no special charaters in path to use a great test runner. :)
npx jest --findRelatedTests src/app/@core/store/store.ts
# jest should try to find a test file at src/app/@core/store/store.(spec|test).ts

npx jest --findRelatedTests src/app/@core/store/store
# jest should try to find all test files in src/app/@core/store/store if a folder exist (src/app/@core/store/store/*.(spec|test).(.js|.jsx|.ts|.tsx)
# if no folder exist or no spec files found in the existing folder, jest should try to find a test file at src/app/@core/store/store.(spec|test).(js|jsx|ts|tsx)

@ahnpnl
Copy link
Contributor

ahnpnl commented Jun 6, 2021

I like the suggestion.

cc @SimenB

@blephy
Copy link
Author

blephy commented Jun 6, 2021

If it's ok for you, in my opinion, you will need to compute a regExp based on the path entry, but the hard work is to include the jest config, which permit to specified a test file pattern if i remember correctly. And the other hard work is to not run any same files which is included in different paths for the same commands.

@nicojs can you give us your Stryker expertise ? If it this contract is respected, is Stryker jest runner should work better too ?

Facebook, If you need some help, i would be glad ;)

@blephy
Copy link
Author

blephy commented Jun 6, 2021

Just for your team, this issue is also related to #11527

@nicojs
Copy link
Contributor

nicojs commented Jun 7, 2021

@blephy Thanks for all your hard work on this issue 🎉

StrykerJS uses --findRelatedTests to speed up jest test execution during mutation testing considerably (since StrykerJS knows exactly which file is mutated at a particular time, it specifies that file with --findRelatedTests), but you can turn this behavior off using { "jest": { "enableFindRelatedTests": false } }, so that is the current workaround for projects that have issues. See https://stryker-mutator.io/docs/stryker-js/jest-runner#configuration.

@blephy's suggestion would work for us. We mostly need a "bugfree" version of --findRelatedTests. We only use the "full file path" (incl extension), so that would work.

However, from Jest's point of view, it would be technically impossible to know the difference between path that includes a file extension vs one that doesn't without performing disk IO. For example, "foo.bar" might refer to a file with name "foo" with extension ".bar", or it could refer to a directory "foo.bar" which contains files. I would suggest the following:

  • If the path contains "magic" (based on "testMatch" rules, see https://jestjs.io/docs/configuration#testmatch-arraystring)
    • Resolve the paths to directory entries (using node-glob or similar implementation)
  • For each listed path:
    • Resolve to a directory entry.
      • If the directory entry resolves to a directory, search recursively in that directory for files with supported extensions that DO NOT match the testPattern regex (Note: this would be optional for StrykerJS).
      • If this directory entry resolves to a file, use that
  • Run "findRelatedTest" lookup on all the resolved file paths and execute those files.

@blephy
Copy link
Author

blephy commented Jun 7, 2021

@blephy Thanks for all your hard work on this issue 🎉

You're welcome

StrykerJS uses --findRelatedTests to speed up jest test execution during mutation testing considerably (since StrykerJS knows exactly which file is mutated at a particular time, it specifies that file with --findRelatedTests), but you can turn this behavior off using { "jest": { "enableFindRelatedTests": false } }, so that is the current workaround for projects that have issues. See https://stryker-mutator.io/docs/stryker-js/jest-runner#configuration.

That's why stryker takes 2 days to run on my repository (29K lignes of es6 code without comments / spaces, cpu runner free of other task) because in my case i must use { "jest": { "enableFindRelatedTests": false } } 😭 .

@nicojs
Copy link
Contributor

nicojs commented Jun 7, 2021

While we're at it: #9728 is also a big hurdle for StrykerJS users 😇

@nicojs
Copy link
Contributor

nicojs commented Jun 8, 2021

I've noticed that this only seems to affect Windows.

On Linux (WSL):

$ npx jest --findRelatedTests src/@core/my-lib.ts
 PASS  src/@core/my-lib.spec.ts
  Square method
    ✓ should be defined (2 ms)
    ✓ should return right value (3 ms)
    

$ npx jest --findRelatedTests 'src/@$_(ok)/my-lib.ts'
 PASS  src/@$_(ok)/my-lib.spec.ts
  Square method
    ✓ should be defined (2 ms)
    ✓ should return right value (2 ms)

I'll try to debug to see what's going on here.

@nicojs
Copy link
Contributor

nicojs commented Jun 8, 2021

I've found the issues. It has to do with the way file patterns are handled on windows. I've implemented a fix in #11548 (which also fixes #9728). This merely fixes the bug, it doesn't add support for targeting directories.

@blephy
Copy link
Author

blephy commented Jun 8, 2021

@nicojs you rock 🥰

@blephy
Copy link
Author

blephy commented Jul 8, 2021

Just for posterity :

Here is an update of the repository which help us to demonstrate the issue. I'm linking the CI which runs on multiple OS and node versions.

The bug is fixed but it seems that --findRelatedTests with special character like ( needs to be encapsulated in quote.

Thanks for the work.

@nicojs
Copy link
Contributor

nicojs commented Jul 8, 2021

Cool! Thanks for checking it out @blephy

@blephy
Copy link
Author

blephy commented Jul 8, 2021

You're welcome

@github-actions
Copy link

github-actions bot commented Aug 8, 2021

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 Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants