fix: begin running tests immediately instead of waiting for watcher

[jedwards1211]

PR Checklist

• Addresses an existing open issue: fixes 🚀 Feature [watch mode]: begin running tests immediately instead of waiting for watcher ready event #5372
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

Turn off ignoreInitial and distinguish between initial 'add' events and paths added after test startup. We can compare the mtime of the path to the test startup time to decide if the change should trigger a rerun.

[jedwards1211]

@mark-wiemer is using new Date() allowed in the CLI code? An ESLint rule complains about it. If not is there any way I can get the current time to compare to file timestamps?

Also, let me know if you have any opinions about if/how I should test this change. It would be nice if I could inject some kind of chokidar mock so I can control timing of the file events to be able to verify in a test that it runs before the ready event is emitted...but the only easy way I think of to do that would be to call things in watch-run.js directly from the test process instead of executing mocha --watch in a separate process.

[mark-wiemer]

The earlier versions of Mocha using Chokidar 3 still waited until watcher.on('ready') to call mocha.runGlobalSetup(), but I'm not sure why. A quick look at the code isn't too revealing, but I see why we'd want to call this right away. I'm curious if we can add a test case that has a complex global setup to ensure things still work?

[jedwards1211]

I'm not exactly familiar with what that global setup is (loading required modules?) but I can try to figure out what goes into it

[jedwards1211]

Okay Chokidar doesn't seem to specify whether 'change'/'unlink' events can get emitted before 'ready' if changes occur during its initial scan, but if so, it could cause the preexisting code here to start running tests before it's performed global setup.

Obviously the intention of the preexisting code is to wait for the 'ready' event to run everything for the first time (first global setup, then the first test run).

I've fixed this in my PR now, if it gets an event before the global fixture context has been created it won't schedule a rerun.

(I still need to write tests for this)

[mark-wiemer]

Marking as draft until tests are added

[jedwards1211]

@mark-wiemer I want to run some changes by you before I dive into them...

I realized that GlobFilesTracker synchronously globs for the watched/ignored files, which mostly defeats the purpose of starting tests before the watcher is ready, since all that globbing would take about as long. So I think we should make it glob asynchronously.

Also...

• It globs each watch pattern and each ignore pattern separately...but it seems like we can just pass all of the watch and ignore patterns to one glob call?
• The existing code re-globs everything when the watcher gets a path added event, which could be a lot of redundant work...instead I think we should check if the added path is already in the watched or ignored set, and if not and it's a directory, initiate a glob on just that directory?

[mark-wiemer]

Hey @jedwards1211, the current glob code is not ideal for sure for exactly the reasons you mentioned and probably more. The best discussion around this is #5379 and in short we don't want to touch this code until v12. We as maintainers don't have a ton of experience with glob matching and don't want to break anything, but we do want to keep Chokidar at v4. Mocha 11.2.1 used Chokidar v3, we upgraded to v4 in Mocha 11.2.2, but Chokidar v4 dropped glob support so we had to use our own. Learning more about globs is definitely a priority, but I'm still getting the ropes on the rest of Mocha as a new maintainer, so I won't have a ton of updates here.

In short, feel free to update or change or even rewrite the glob handling. Just keep #5379 in mind as another alternative and add as many tests as you can to help me and other maintainers verify any changes :)

Related issues include #5355 and #5374

[jedwards1211]

Yeah I kind of thought about doing the same thing as that PR.

One question is how to change blastCache - if we start and possibly even rerun before the watcher is finished scanning, we can't rely on getting the watched paths from chokidar. I already had to fix this in this PR, by getting the paths to delete from the GlobFilesTracker.

Instead we would have to delete all require.cache paths matching the filter. I worry a bit about how well that would perform compared to just deleting the list of matched paths we know about though. If there's a long list of watch patterns it could be a lot of work determining matches.

[mark-wiemer]

I wouldn't worry about performance too much in the abstract. We had a concrete issue with #5374 that has effectively been resolved with #5379, and in practice I can't imagine folks are using many regexes for watch paths. If we get a bug report about perf, we can look into it, but as long as our basic stress tests pass, I'd prefer simpler changes first.

On another note, @mochajs/maintenance-crew I'm thinking of marking this as "semver-major" given the complexity impacted and possibility of bugs. Since we want Mocha 12 to have minimal changes we'll probably hold this one for Mocha 13. (#5357)

[jedwards1211]

Okay I guess I'll eventually piggyback this on #5379 and blast the cache with the new filtering from that PR.

Also, I'm not sure what to do about getting the launch time to compare file mtimes to. Date etc could get monkeypatched by test utils like Sinin fake timers... do you have any thoughts?

I guess it won't get monkeypatched before we run global setup though. But I got an ESLint error about using Date, it must be for good reason.

[mark-wiemer]

I'm not sure why our linter warns about using Date. Could you see if that's a recommended rule? There's a real chance that our linter is very out of date. We've had some discussions about it but haven't prioritized updates there for a while.

[jedwards1211]

@mark-wiemer looks like the rule was explicitly added to guard against exactly the danger of fake timers I was talking about: #237

(This issue is explicitly linked in your ESLint config)

Note, it would probably be possible to grab a reference to the original Date constructor, Date.now() before it can get monkeypatched, and use that if necessary throughout Mocha.

As I was saying though, in this case I only need to get the launch date before running any global setup where userland code might monkeypatch Date, so I guess it's okay to suppress the error in this case

[mark-wiemer]

Thanks for researching. The core concern still applies, but I think the workaround does too: just have Date = global.Date at the top-level and then call Date() within your function as needed. Sinon mocks global.Date once the tests start running, but if we do this at top-level, it's done before the tests are run. As long as we never call global.Date() within a function we are good. I think... once the code is ready I'll definitely ping other maintainers to review it!

[JoshuaKGoldberg]

👋 Just checking in @jedwards1211, are you waiting on us for anything?

[jedwards1211]

No, I'll pick this back up soon now that that related PR is merged

[jedwards1211]

@mark-wiemer okay, to test timing-sensitive cases, I used an IPC channel to send mock chokidar events and tell the mocha process when to pretend global setup is done. I turned this mode on by passing a MOCHA_TEST_WATCH environment variable, but I'm not sure if you'd want this...if not how do you want me to accomplish the mocking?

[codecov]

Codecov Report

❌ Patch coverage is 97.77778% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.70%. Comparing base (f75d150) to head (383dbb1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/cli/watch-run.js	97.77%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5409      +/-   ##
==========================================
+ Coverage   93.69%   93.70%   +0.01%     
==========================================
  Files          57       57              
  Lines        4396     4419      +23     
  Branches      849      860      +11     
==========================================
+ Hits         4119     4141      +22     
- Misses        277      278       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mark-wiemer]

@jedwards1211 new tests are frequently timing out, do you think we can refactor them to complete in under 10 seconds?

[jedwards1211]

@mark-wiemer huh, there may be a race condition causing the new tests to time out, I'll see what I can figure out. Theoretically they should run especially quickly since they aren't designed to rely on any arbitrary delays. (Many of the watch tests do use delays of a second or so, which makes running the entire watch test suite quite slow).

[jedwards1211]

@mark-wiemer is there a way I can run CI with SSH to debug if the tests keep timing out? It's not happening to me locally

[jedwards1211]

oh I think I know what was causing the timeouts, they were probably sometimes missing the initial { listening: true } event sent over IPC because of the default sleepMs: 2000 before running the test-supplied change function. I set sleepMs: 0 in my tests now

[mark-wiemer]

@jedwards1211 seems the tests are consistently failing on Windows, are you able to look into it?

[jedwards1211]

Yup, I will tomorrow

[jedwards1211]

@mark-wiemer is there a way I can run the Windows CI tests with SSH? I can't reproduce locally on a Windows machine. It seems to be some issue with IPC on Windows CI, but I'll need to either SSH into the tests or turn on some kind of debug logging to figure it out.

[mark-wiemer]

I'm going to re-run the tests just to make sure this wasn't some weird fluke. And if they fail again, I'll try to repro from Windows on my side. I'll note that you can run the tests locally with the DEBUG flag on (ref #5538) , but if you can't repro, I understand those debug logs don't mean much.

Ref https://github.com/mochajs/mocha/actions/runs/19623994930 for test run

[jedwards1211]

@mark-wiemer okay I temporarily turned on more debug logging in these tests, hopefully that will help me figure out what's going on in Windows CI

[jedwards1211]

@mark-wiemer lol, the Windows issue is that os.tmpdir() seems to return a goofy 8.3 name:

2025-11-24T17:33:44.460Z mocha:cli:watch clean path: C:\Users\RUNNER~1\AppData\Local\Temp\mocha-JJqlNx\.git
2025-11-24T17:33:44.460Z mocha:cli:watch absolute path: C:\Users\RUNNER~1\AppData\Local\Temp\mocha-JJqlNx\.git

In the new watch tests I'm realpath-ing temp file paths for the mock watcher events because on macOS at least, os.tmpdir() contains symlinks. Well, in Windows, that resolves C:\Users\RUNNER~1 to C:\Users\runneradmin:

2025-11-24T17:33:44.469Z mocha:cli:watch got message {
  watcher: [
    'all',
    'add',
    'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\mocha-JJqlNx\\test.js'
  ]
}

I changed the createTempDir helper to realpath the directory it returns for all tests that use it instead, hopefully it should solve all these problems and not break anything else.

I guess I should also confirm that if Chokdiar is watching the contents of C:\Users\RUNNER~1 on Windows, it also uses the 8.3 name in the file paths it emits, instead of the realpath.

Also -- I think all require.cache entries are realpaths, but I need to double check. We're deleting modules from require.cache that match the watch path filters before rerunning, so if one is a realpath and the other isn't, it won't work as intended.

[jedwards1211]

Okay this stuff about Windows short names is really aggravating. Here's what I've discovered...

• require('fs').realpathSync('C:\\Users\\RUNNER~1') returns C:\Users\RUNNER~1
• require('fs').realpathSync.native('C:\\Users\\RUNNER~1') returns C:\Users\runneradmin
• require.cache does contain 'C:\\Users\\RUNNER~1\\...' entries when the CWD is C:\Users\RUNNER~1
• Fun fact: require('fs/promises').realpath is actually based upon require('fs').realpath.native. I'm not the only one who found this confusing

I guess leaving the fs/promises realpathed temp dirs for testing should work, but maybe I should also add tests that don't realpath it on windows...