🚀 Feature: Change default value for --ignore to **/node_modules, **/.git

[Danielku15]

Feature Request Checklist

• I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
• I have searched for related issues and issues with the faq label, but none matched my issue.
• I want to provide a PR to resolve this

Overview

Looking at the current behavior of Mocha there is an inconsistency between normal execution and watch execution due to different defaults in --ignore and --watch-ignore.

I think many devs will agree that picking up data from the node_modules folder is not in their interest when maintaining any project and running tests.

Additionally the node_modules folder might live in a subfolder (mono repos) but still is subject of being ignored. The current pattern only ignores node_modules on the level of the mocharc config file.

Suggested Solution

1. The default values for the two options should be set to the same value.
2. The default patterns should also ignore these folders if they are in subfolders.

Hence the new default for both options should be similar to: ['**/node_modules/', '**/.git/']

Alternatives

If this is the intended behavior and has a reasoning, this reasoning should be documented on the website.

Additional Info

When using NPM workspaces (common in mono-repos) there will be symlinks from node_modules to the directories of the packages. https://docs.npmjs.com/cli/v10/using-npm/workspaces
Running Mocha with --reporter=json then shows that rather the tests in the node_modules are picked up than the real working directory.

This can lead to further problems when processing the test reports or in case external tooling relies on these paths.

The problem became visible while working on a VS Code Extension and tests from node_modules were listed.

CoderLine/mocha-vscode#1

[JoshuaKGoldberg]

Ok coming back to this: I think I better understand things now, and am not in favor of unifying the two just yet. It's feasible that a project could want to have a separate glob for test files (--ignore) as opposed to watching files (--watch-ignore). For reference, Jest has separate options for testPathIgnorePatterns vs. watchPathIgnorePatterns. Given that this is the only issue I've seen about separating the two options, let's hold off on unifying them for now.

However, changing the default values makes a lot of sense to me. I think the values were first set before JavaScript monorepos were widespread & popular. Accepting PRs for the single change of setting the defaults of both --ignore and --watch-ignore to be the recursive ['**/node_modules/', '**/.git/'].

Note that this would be a semver-major breaking change. Which means we wouldn't be able to merge until we're working on Mocha 12.

Edit: ah, #5203 exists to track --watch-ignore switching to the recursive form. So let's use this issue to track changing --ignore to default to the recursive ['**/node_modules/', '**/.git/'].

[Danielku15]

Ok coming back to this: I think I better understand things now, and am not in favor of unifying the two just yet. It's feasible that a project could want to have a separate glob for test files (--ignore) as opposed to watching files (--watch-ignore). For reference, Jest has separate options for testPathIgnorePatterns vs. watchPathIgnorePatterns. Given that this is the only issue I've seen about separating the two options, let's hold off on unifying them for now.

Looks like there was some sort of mistunderstanding. In my original post I had proposed to only unify the default values of the options. My intention was never to have only one option. The initial title was badly phrased and a bit misleading, the body represents better my intentions. I agree that it makes sense to have two options and I don't see a need to change Mocha to only have one.

However, changing the default values makes a lot of sense to me. I think the values were first set before JavaScript monorepos were widespread & popular. Accepting PRs for the single change of setting the defaults of both --ignore and --watch-ignore to be the recursive ['**/node_modules/', '**/.git/'].

This matches my proposed solution. 👍 from my side to keep both options and change their defaults to the same pattern as noted.

Note that this would be a semver-major breaking change. Which means we wouldn't be able to merge until we're working on Mocha 12.

Edit: ah, #5203 exists to track --watch-ignore switching to the recursive form. So let's use this issue to track changing --ignore to default to the recursive ['**/node_modules/', '**/.git/'].

Fine for me. Not sure how long the road to Mocha 12 is, until then maybe some few docs on that inconsistency and plan could help avoiding problems. This will also help in cases where people might report bugs to the vscode extension.

[JoshuaKGoldberg]

proposed only to unify the default values

Ah great! Sorry for misinterpreting then 🙂.

until then maybe some few docs on that inconsistency and plan could help avoiding problems. This will also help in cases where people might report bugs to the vscode extension.

Great idea. I'd 👍 a PR that adds notes to the mochajs.org docs.