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

Escape glob characters in rootDir before interpolating into testMatch #5224

Merged
merged 3 commits into from
Jan 6, 2018

Conversation

rimunroe
Copy link
Contributor

@rimunroe rimunroe commented Jan 3, 2018

Summary

This is an attempt to address #4838 and facebook/create-react-app#3404. Currently, some tests aren't found when the user is running Jest in a project where the path contains parentheses and they have the <rootDir> token in one of the testMatch strings. This seems to be because testMatch is treated as an array of Globs, but rootDir is dropped into those Globs unaltered.

For an example, given this configuration:

{
  "rootDir": "/(parens)/",
  "testMatch": [
    "<rootDir>**/*.test.js"
  ]
}

Jest will end up with /(parens)/**/*.test.js as a testMatch pattern. This will match the file at /parens/src/index.test.js, rather than file /(parens)/src/index.test.js, since micromatch will treat the parens as a designating a regex group containing a single pattern.

It looks like this is also a problem in every other place in the configuration where <rootDir> would be used in a glob pattern. I tried addressing the issue in those places too, but this ended up causing some new test failures, so I'm just pushing up my attempt at fixing the immediate problem. It would be nice if there were a way to address the other cases here too. Is there a use-case for having a rootDir glob pattern instead of just a single path?

Test plan

I added an additional failing test before beginning, though I had trouble finding a place for it that seemed suitable and ended up just putting it near some tests that seemed to be checking for similar things inside jest-cli.

This is how I checked to see if the changes addressed #4838:

  1. Build Jest from this branch.
  2. yarn link inside packages/jest-cli.
  3. Clone my example project (https://github.com/rimunroe/jest-parentheses-in-rootDir) into some directory.
  4. cd into to that directory, and then into ./with-(parens).
  5. Run yarn to install the un-linked Jest.
  6. Run yarn test and note that no tests were found.
  7. Run yarn link jest-cli to use the new build of Jest instead.
  8. Run yarn test again, which should result in a test being found.

@codecov-io
Copy link

Codecov Report

Merging #5224 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5224      +/-   ##
==========================================
+ Coverage   60.91%   60.93%   +0.02%     
==========================================
  Files         202      202              
  Lines        6731     6733       +2     
  Branches        4        3       -1     
==========================================
+ Hits         4100     4103       +3     
+ Misses       2630     2629       -1     
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-config/src/normalize.js 93.1% <100%> (ø) ⬆️
packages/jest-config/src/utils.js 88% <100%> (+2.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96a44df...78a6f3a. Read the comment docs.

@cpojer
Copy link
Member

cpojer commented Jan 4, 2018

Interesting. I think we didn't intend glob patterns to use <rootDir> at all, so maybe instead we could change this to throw and update CRA not to use this when ejecting?

@rimunroe
Copy link
Contributor Author

rimunroe commented Jan 5, 2018

The documentation makes it seem like it's at least somewhat expected. For example, there's an explicit example of using <rootDir> in glob patterns in the section on projects:

projects [array]

Default: undefined

When the projects configuration is provided with an array of paths or glob
patterns, Jest will run tests in all of the specified projects at the same time.
This is great for monorepos or when working on multiple projects at the same
time.

{
  "projects": ["<rootDir>", "<rootDir>/examples/*"]
}

This example configuration will run Jest in the root directory as well as in
every folder in the examples directory. You can have an unlimited amount of
projects running in the same Jest instance.

Also, the version of that section currently up on the Jest website has another bit showing using <rootDir> in a glob pattern for testMatch too:

The projects feature can also be used to run multiple configurations or multiple runners. For this purpose you can pass an array of configuration objects. For example, to run both tests and ESLint (via jest-runner-eslint) in the same invocation of Jest:

{
  "projects": [
    {
      "displayName": "test"
    },
    {
      "displayName": "lint",
      "runner": "jest-runner-eslint",
      "testMatch": ["<rootDir>/**/*.js"]
    }
  ]
}

I guess I'm kind of fuzzy on what rootDir should be for in the first place. My question is: if <rootDir> shouldn't be used in cases where glob patterns are used, then what are the cases where it can be used, and what's the impact on the rest of these options? It seems like forbidding it from glob patterns would be a far-reaching change.

@cpojer
Copy link
Member

cpojer commented Jan 6, 2018

Yeah, you are right about this being confusing. Originally the idea was that any regex pattern in the config would support rootDir (legacy Jest basically) but that globs don't need it. Right now we are in this confusing state where we support both globs and regex for different config options. I think it's best to merge your PR to make this less confusing for users even if technically we don't really need rootDir to be included in glob patterns, it still works.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
@rimunroe rimunroe deleted the escape-glob-characters-in-rootDir branch May 13, 2021 01:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants