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

[new rule] Enforce using jest.doMock if a jest.mock call cannot be hoisted #1315

Open
NotWoods opened this issue Dec 24, 2022 · 1 comment
Open

Comments

@NotWoods
Copy link
Contributor

NotWoods commented Dec 24, 2022

The conditions around when babel-jest can hoist a function is a bit confusing. If mocks are hoisted, then it's safe to require a module that depends on them. But if they aren't, then the real module will be unintentionally used instead.

If tests consistently use jest.mock for hoistable module mocks, and use jest.doMock for mocks that cannot be hoisted, then its easy to just look at the test and see if its safe to require a module that depends on the mocks at the top level.

Here’s the checks I found from digging through the source code of Jest’s babel plugin.

  • jest.mock() must be called at the top-level, and not inside another function.
  • The first argument to jest.mock() should be a literal value.
  • The second argument, if provided, should be an inline function.
  • That function shouldn’t reference any variables defined outside the function.

Examples of incorrect code for this rule:

const moduleName = './module';
jest.mock(moduleName);

function factory() {
  return jest.fn();
}
jest.mock('./module', factory);

jest.mock('./module', () => {
  // variable defined outside scope
  return jest.fn().mockReturnValue(moduleName);
});

Examples of correct code for this rule:

const moduleName = './module';
jest.doMock(moduleName);

function factory() {
  return jest.fn();
}
jest.doMock('./module', factory);

jest.doMock('./module', () => {
  return jest.fn().mockReturnValue(moduleName);
});

// can be hoisted
jest.mock('random-num');
jest.mock('../module', () => {
  return jest.fn().mockReturnValue('foo');
});

Auto-fixable: Yes

@G-Rath
Copy link
Collaborator

G-Rath commented Dec 24, 2022

I'm not sure how valuable/ accurate we could be on this - in my experience mocking beyond jest.mock and jest.spyOn tend to quickly become pretty advanced & complex so I suspect such a rule could have a lot of caveats.

Two specific things that come to mind is that mocking is often done for modules that are imported within other files (which we can't know about), and that there's a rule about allowing variables that begin with mock as a bail-out (which acknowledges how complex mocking can be).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants