Parallel mode fails silently sometimes with tests that import mocha

[giltayar]

Prerequisites

• Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
• 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
• Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

IF you're using --parallel
AND the number of test files > number of workers (i.e. a worker is reused to run two or more test files)
AND you're using import {it} from "mocha" to get your it (note that this is not a problem in CJS)
THEN the subsequent test files that run in the reused worker will silently not run.

For example if the number of worker is 1, and you're running two test files, the first test file will have its tests executed, but the second test file will load, but the tests won't run.

Steps to Reproduce

Write two test files that use import {it} from "mocha" and use that it to run tests.

Now run them using mocha file1.mjs file2.mjs using --parallel in a machine with just one CPU. Note that --job 1 does not reproduce it. Only a machine with 1 CPU. And this is a problem mostly in CI, because dev machines usually have more than 1 CPU.

Expected behavior: All tests should have been executed.

Actual behavior: Only the tests in the first file are executed. The tests of the second file will be ignored.

Reproduces how often: consistent. Always.

Versions

• The output of mocha --version and node node_modules/.bin/mocha --version: 8.2.1
• The output of node --version: 15.6.0
• Your operating system
  • name and version: macOS Big Sure
  • architecture (32 or 64-bit): 64
• Your shell (e.g., bash, zsh, PowerShell, cmd):
• Your browser and version (if running browser tests):
• Any third-party Mocha-related modules (and their versions): nope
• Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): nope

Additional Information

I dug into the code. The problem is this:

• Mocha overrides the global.it so that when it is called, tests are added to the current suite (in bdd.js in the bddInterface function. This also happens in the other interfacs). This is done in the PRE_REQUIRE phase of the tests, before the first test file is loaded.
• The first test file is loaded and when const {it} = require('mocha') is called, it receives this global.it.
• The suite gets the test functions and all is well.
• Now the second test file arrives at the same worker, which creates a new Mocha and thus a new suite.
• Again, in the pre-require, the global.it is overriden, so that when called, it will add tests to the new suite.
• Again, the test file is loaded, but this time, when import {it} from 'mocha' is called, the MJS cache happens and the test gets the original it, which adds the tests to the original suite and not the new one.
• Now when we get to executing tests, there are no tests to execute, and we're done.

The problem is that the Mocha class is treated as a class that can be instantiated multiple times, when it is in fact a singleton due to its use of global.it.

My guess is that this does not happen in CJS because in CJS you clear the CJS module cache before loading the files. Unfortunately, ESM does not have a way to clear the cache.

Solutions:

1. Clear the cache. This will work in CJS, but not in ESM that has no API (documented or otherwise) to clear the cache.
2. Make workers non-reusable.
3. Make it use a "current" suite that is global and set each time a new Mocha is instantiated.

I prefer solution 2, because this problem may just be the tip of the iceberg in terms of problems that happen because Mocha instantiated multiple times.

One way to implement solution 2 is to use workerpool with numworkers=1 and implement the parallelism by throttling calls to workerpool.exec by using a throttling library like throat.

[giltayar]

Yesterday night I realized there is a fourth option to fix it (a variation on solution 3 above). The solution is to make the test functions (e.g. it, describe) be a proxy to Mocha.it/describe/*. That way, calling the entry point it will proxy to the current Mocha.it and all will be well.

This was implemented in the PR that creates ESM entry points, and all the tests pass without a problem.

I like this solution because it doesn't try to re-architect Mocha, but rather is a local solution that solves the problem at hand: ESM entry points. On the way, we solve it for CJS entry points as well.

[nicojs]

Hi giltayar, thanks for this extensive explanation!

I've been trying to reproduce this issue, but I can't seem to do it. Please see attached zip file
parallel.zip. Unzip and npm i + npx mocha. It runs mocha in --parallel mode with --jobs 2. The 3rd test file should result in a failure (it throws) and it does. So it is not silently ignored.

Any help with reproducing it would be much appreciated.

[giltayar]

@nicojs Thanks for taking the time for this. I downloaded your zip and tried it, and you were right. Using npx test*.* --parallel --jobs 1 does not reproduce the problem.

So I did two things:

1. Converted the js files to mjs with import. Still didn't reproduce.
2. Ran it on a machine with 1 CPU (I used the <https://multipass.run/ VM>. Reproduced, even without --jobs 1!

So I modified the bug description accordingly. The bug description changes were:

1. This happens only on ESM.
2. This happens only with 1 CPU (or, my guess is that it happens when number of CPUs < number of test files).

Note that I have a commit that fixes the problem according to my suggestion (see the commit referenced in this conversation), so I believe that what I think is the problem is indeed that.

[nicojs]

Thanks, I was able to reproduce it now (with --jobs 2, 8 core CPU). I think your suggestions for making describe, it, etc proxies make the most sense, I was actually a bit surprised they weren't already.

[giltayar]

Good to know. I have a fix in the PR I mentioned above, but that PR has been lagging for a while (somewhat controversial and complex, it seems...), so I'm happy it's getting a fix outside of that PR. I'll probably have to deal with conflicts with it once it's merged. Oh, well, it's all good.

[nicojs]

I have a fix in the PR I mentioned above

Yeah, I've taken a look and based my PR on it. I was wondering why a lot of files needed changing. Only proxying from mocha.js seems enough. Do you agree?

[giltayar]

Yeah, I've taken a look and based my PR on it. I was wondering why a lot of files needed changing. Only proxying from mocha.js seems enough. Do you agree?

I do. Your solution is much simpler.