Skip to content

fix(valid-mock-module-path): don't report virtual mocks#1946

Merged
G-Rath merged 8 commits into
jest-community:mainfrom
Xunnamius:fix-valid-mock-module
Apr 9, 2026
Merged

fix(valid-mock-module-path): don't report virtual mocks#1946
G-Rath merged 8 commits into
jest-community:mainfrom
Xunnamius:fix-valid-mock-module

Conversation

@Xunnamius
Copy link
Copy Markdown
Contributor

Fixes #1945

@Xunnamius Xunnamius force-pushed the fix-valid-mock-module branch from ebade85 to c16102c Compare March 22, 2026 14:56
Comment thread src/rules/valid-mock-module-path.ts Outdated
@Xunnamius Xunnamius force-pushed the fix-valid-mock-module branch from c16102c to 9cdb5e6 Compare March 22, 2026 15:02
@G-Rath G-Rath changed the title fix(rules/valid-mock-module-path): prevent false positive when mocks are virtual fix(valid-mock-module-path): don't report virtual mocks Mar 22, 2026
Copy link
Copy Markdown
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're missing a bunch of cases - what if...

  • an array or function is passed in as the param
  • a variable is passed as the param
  • an object is passed, without any properties
  • an object is passed, which has a spread
  • an object is passed, with a property that is not virtual
  • an object is passed, with a computed property
    • ...whose key comes from a variable
    • ...whose key is a string literal...
      • ...which is virtual
      • ...which is not virtual
    • ...whose key is a template string literal...
      • ...which is virtual
      • ...which is not virtual
      • ...which has interpolation
  • an object is passed, with the virtual property but whose value...
    • ...is a variable
    • ...is a variable, and is shorthanded
    • ...is an array or function
    • ...is a number

Within these cases, you should find your missing coverage 🙂

Comment thread src/rules/valid-mock-module-path.ts Outdated
@Xunnamius
Copy link
Copy Markdown
Contributor Author

Xunnamius commented Mar 22, 2026

you're missing a bunch of cases - what if...

I assume this is a response to my "a test case that can trigger the else path" question? If so, I'll see if one of these triggers the else path. Many of these I've already considered (computed property, passing function, object spread, etc) 🙂

@G-Rath
Copy link
Copy Markdown
Collaborator

G-Rath commented Mar 22, 2026

Yes and no - these are all tests that you should have, because most of them have different AST shapes, and I know they will cover your else branches (specifically, spread is the test you're missing for coverage as properties is type ObjectLiteralElement which is an alias for Property | SpreadElement, but we still want the majority of the situations I've mentioned explicitly tested)

@Xunnamius
Copy link
Copy Markdown
Contributor Author

Spread was literally the first thing I tried, haha. However, I'm getting a syntax error from your test pipeline when I include it. Is there something specific I should add there to enable that syntax?

As for the other test cases, looking at the existing tests for this rule, I tried to match their level of exhaustiveness. Unfortunately I'm approaching the end of my timebox for this, so I can't go much further beyond that this week

@Xunnamius
Copy link
Copy Markdown
Contributor Author

Xunnamius commented Mar 22, 2026

However, I'm getting a syntax error from your test pipeline when I include it.

Looks like this:

valid-mock-module-path › invalid › jest.mock('../module/does/not/exist', undefined, { ...{ v: true } })

    assert(received)

    Expected value to be equal to:
      true
    Received:
      false
    
    Message:
      A fatal parsing error occurred: Parsing error: Unexpected token ...

      at runRuleForItem (node_modules/eslint/lib/rule-tester/rule-tester.js:894:4)

(I'm coming from babel land, where it's usually an incantation/config property/package to get syntaxes working)

@Xunnamius Xunnamius requested a review from G-Rath March 22, 2026 19:20
Xunnamius added a commit to Xunnamius/symbiote that referenced this pull request Mar 22, 2026
@G-Rath
Copy link
Copy Markdown
Collaborator

G-Rath commented Mar 25, 2026

I'm getting a syntax error from your test pipeline when I include it.

You want to adjust the configuration being passed to RuleTester, specifically the ecmaVersion - I believe you want 2015:

const ruleTester = new RuleTester({
parser: espreeParser,
parserOptions: {
ecmaVersion: 2015,
},
});

@G-Rath G-Rath merged commit a1916d1 into jest-community:main Apr 9, 2026
28 checks passed
github-actions Bot pushed a commit that referenced this pull request Apr 9, 2026
## [29.15.2](v29.15.1...v29.15.2) (2026-04-09)

### Bug Fixes

* **valid-mock-module-path:** don't report virtual mocks ([#1946](#1946)) ([a1916d1](a1916d1))
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2026

🎉 This PR is included in version 29.15.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[valid-mock-module-path] false positive when using virtual mocks

2 participants