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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

jest --only-changed ignores changes to module mocks #8907

Closed
tomasz-sodzawiczny opened this issue Sep 3, 2019 · 7 comments
Closed

jest --only-changed ignores changes to module mocks #8907

tomasz-sodzawiczny opened this issue Sep 3, 2019 · 7 comments

Comments

@tomasz-sodzawiczny
Copy link

馃悰 Bug Report

jest --only-changed does not take mocks into consideration when choosing the tests to run.

To Reproduce

Steps to reproduce the behavior (see example repo):

  • module ./a depends on module ./b
  • module ./b is mocked in the test of './a`
  • you change the mock of module ./b

Expected behavior

Test of module ./a is run when running jest --only-changed.

The success of the test depends on changes of the mock file - so changing the mock should trigger the test - the same way as changing ./b directly does.

The scenario is not uncommon - could happen when someone is extending the functionality of ./b, or works on another module that depends on it.

Link to repl or repo

https://github.com/tomasz-sodzawiczny/jest-only-changed-vs-mocks

envinfo

Probably not important, but:

System:
    OS: macOS 10.14.5
    CPU: x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
  Binaries:
    Node: 12.4.0 - ~/.nvs/node/12.4.0/x64/bin/node
    Yarn: 1.17.3 - /usr/local/bin/yarn
    npm: 6.9.0 - ~/.nvs/node/12.4.0/x64/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0
@SimenB
Copy link
Member

SimenB commented Sep 3, 2019

The code for this lives in https://github.com/facebook/jest/blob/c588c18595000d68c98bf8c72d003ff8b3674124/packages/jest-resolve-dependencies/src/index.ts

A PR fixing this is very much welcome! 馃檪

@petevdp
Copy link
Contributor

petevdp commented Sep 12, 2019

I'm a first time contributer and I'd like to tackle this, but if anyone has the time I'd like some initial feedback on how I should tackle this problem.

I've done some digging and I've found this file dependencyExtractor, which has some regex patterns it uses to find dependency declarations, indirectly supplying the resolver in the file linked by @SimenB with dependencies for files.

Other patterns matching jest.requireMock are present, but not jest.mock.

dependencyExtractor also doesn't include the mocks themselves as dependencies, only the files that are being mocked. I'm guessing that there's a reason it's like this but maybe not.

So with all that in mind, would a viable solution be to expand the regexes and add the mock files as dependencies where applicable?

If not, some alternatives would be to:

  • Alter the output signature of the extractor to seperate mock dependencies, and the fileMetaData type and any associated types/
  • Change DependencyResolver.resolve to cross reference any mocks known to the resolver against dependencies being checked, and including matching mocks without using any additional file metadata. This would have the drawback of including mocks as dependencies which aren't actually utilized, but is probably the solution with the fewest side effects.

Any feedback is welcome!

@jeysal
Copy link
Contributor

jeysal commented Sep 12, 2019

It's hard to tell without trying it out and seeing what else might break, but I believe changing jest-resolve-dependencies is the cleanest way.

including mocks as dependencies which aren't actually utilized

I think this is tolerable, and will only rarely cause tests being run unnecessarily in practice. I also believe that it's very hard to avoid, because finding out if a mock is used is not easy (it could be declared to be used in lots of places).

@petevdp
Copy link
Contributor

petevdp commented Sep 12, 2019

Okay, I'll give that a try then, thanks!

@brapifra
Copy link
Contributor

I see an open PR for this. Any updates? @petevdp @SimenB
I'd love to see this merged soon 馃檪

@SimenB
Copy link
Member

SimenB commented Oct 27, 2020

#10713

@SimenB SimenB closed this as completed Oct 27, 2020
@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

Successfully merging a pull request may close this issue.

5 participants