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

add support for running tests in parallel #4245

Merged
merged 1 commit into from Jun 1, 2020

Conversation

boneskull
Copy link
Member

@boneskull boneskull commented Apr 21, 2020

(this PR depends on most other PRs linked to #4198, so they should be merged first; documentation will be in another PR)

Summary

This PR adds support for running test files in parallel via --parallel. For many cases, this should "just work."

How It Works

When the --parallel flag is supplied, Mocha will swap out the default Runner (lib/runner.js) for BufferedRunner (lib/buffered-runner.js).

BufferedRunner extends Runner. BufferedRunner#run() is the main point of extension. Instead of executing the tests in serial, it will create a pool of worker processes (not worker threads) based on the maximum job count (--jobs; defaults to <number of CPU cores> - 1). Both BufferedRunner and the worker module consume the abstraction layer, workerpool.

BufferedRunner#run() does not load the test files, unlike Runner#run(). Instead, it has a list of test files, and puts these into an async queue. The EVENT_RUN_BEGIN event is then emitted. As files enter the queue, BufferedRunner#run() tells workerpool to execute the run() function of the pool. workerpool then launches as many worker processes are needed--up to the maximum--and executes the run() function with a single filepath and any options for a Mocha instance.

The first time lib/worker.js is invoked, it will "bootstrap" itself, by handling --require'd modules and validating the UI. Note that reporter validation does not occur. Once bootstrapped, it instantiate Mocha, add the single file, swap any reporter out for the Buffered reporter (lib/reporters/buffered.js) then execute Mocha#run(), which invokes Runner#run().

The Buffered reporter listens for events emitting from the Runner instance, like a reporter usually does. But instead of outputting to the console, it buffers the events in a queue. Once the file has completed running, the queue is drained: the events collected are (trivially) serialized for transmission back to the main process. BufferedRunner#run() receives the list of events, trivially deserializes them, and re-emits the events to whatever the chosen reporter is (e.g., the spec reporter). In this way, the reporters don't know that the tests were run in parallel. Practically, the user will see reporter output in "chunks" instead of the "stream" of results they usually expect. This method ensures that while the test files run in a nondeterministic order, the reporter output will be deterministic for any given test file.

Once the result (the queue of events) has been returned to the main process, the worker process stays open, but waits for further instruction. If there are more files in BufferedRunner#run()'s queue, workerpool will instruct the worker to take the next file from the list, and so on, and so forth. When all files have executed, the pool terminates, the EVENT_RUN_END event is emitted, and the reporter handles it.

Limitations & Caveats

(this section is pasted from the documentation with minimal edits)

Reporter Limitations

Due to the nature of the following reporters, they cannot work when running tests in parallel:

  • markdown
  • progress
  • json-stream

These reporters expect Mocha to know how many tests it plans to run before execution. This information is unavailable in parallel mode, as test files are loaded only when they are about to be run.

In serial mode, tests results will "stream" as they occur. In parallel mode, reporter output is buffered; reporting will occur after each file is completed. In practice, the reporter output will appear in "chunks" (but will otherwise be identical).

Order is Non-Deterministic

In parallel mode, we have no guarantees about the order in which test files will be run--or what process runs them--as it depends on the execution times of the test files.

Because of this, the following options cannot be used in parallel mode:

  • --file
  • --sort
  • --delay

Test Duration Variability

Because running tests in parallel mode uses more system resources at once, the OS may take extra time to schedule and complete some operations. For this reason, test timeouts may need to be increased either globally or otherwise.

"Bail" is "Best Effort"

When used with --bail (or this.bail()) to exit after the first failure, it's likely other tests will be running at the same time. Mocha must shut down its worker processes before exiting.

Likewise, subprocesses may throw uncaught exceptions. When used with --allow-uncaught, Mocha will "bubble" this exception to the main process, but still must shut down its processes.

Root Hooks Are Not Global

NOTE: This only applies to test files run parallel mode.

A root-level hook is a hook in a test file which is not defined within a suite. An example using the bdd interface:

// test/setup.js
beforeEach(function() {
  doMySetup();
});

afterEach(function() {
  doMyTeardown();
});

When run (in the default "serial" mode) via mocha --file "./test/setup.js" "./test/**/*.spec.js", setup.js will be executed first, and install the two hooks shown above for every test found in ./test/**/*.spec.js.

When Mocha runs in parallel mode, test files do not share the same process. Consequently, a root-level hook defined in test file A won't be present in test file B.

There are a (minimum of) two workarounds for this:

  1. require('./setup.js') or import './setup.js' at the top of every test file. Best avoided for those averse to boilerplate.
  2. Recommended: Define root-level hooks in a required file, using the new (also as of VERSION) Root Hook Plugin system.

No Browser Support

Parallel mode is only available in Node.js.

Migration Checklist

If you find your tests don't work properly when run with --parallel, either shrug and move on, or use this handy-dandy checklist to get things working:

  • ✅ Ensure you are using a supported reporter.
  • ✅ Ensure you are not using other unsupported flags.
  • ✅ Double-check your config file; options set in config files will be merged with any command-line option.
  • ✅ Look for root-level hooks in your tests. Move them into a root hook plugin.
  • ✅ Do any assertion, mock, or other test libraries you're consuming use root hooks? They may need to be migrated for compatibility with parallel mode.
  • ✅ If tests are unexpectedly timing out, you may need to increase the default test timeout (via --timeout)
  • ✅ Ensure your tests do not depend on being run in a specific order.
  • ✅ Ensure your tests clean up after themselves; remove temp files, handles, sockets, etc. Don't try to share state or resources between test files.

Caveats About Testing in Parallel

Some types of tests are not so well-suited to run in parallel. For example, extremely timing-sensitive tests, or tests which make I/O requests to a limited pool of resources (such as opening ports, or automating browser windows, hitting a test DB, or remote server, etc.).

Free-tier butt CI services may not provide a suitable multi-core container or VM for their build agents. Regarding expected performance gains in CI: your mileage may vary. It may help to use a conditional in a .mocharc.js to check for process.env.CI, and adjust the job count as appropriate.

It's unlikely (but not impossible) to see a performance gain from a job count greater than the number of available CPU cores. That said, play around with the job count--there's no one-size-fits all, and the unique characteristics of your tests will determine the optimal number of jobs; it may even be that fewer is faster!

Changeset Notes

  • updated signal handling in bin/mocha to a) better work with Windows, and b) work properly with --parallel to avoid leaving zombie workers
  • docstrings in lib/cli/collect-files.js
  • refactors in lib/cli/run-helpers.js and lib/cli/watch-run.js. We now have four methods:
    • watchRun() - serial + watch
    • singleRun() - serial
    • parallelWatchRun() - parallel + watch
    • parallelRun() - parallel
  • lib/cli/run.js and lib/cli/run-option-metadata.js: additions for new options and checks for incompatibility
  • add lib/reporters/buffered.js (Buffered); this reporter is not re-exported in Mocha.reporters, since it should only be invoked internally.
  • tweak landing reporter to avoid referencing Runner#total, which is incompatible with parallel mode. It didn't need to do so in the first place!
  • the tap reporter now outputs the plan at the end instead of at the beginning (avoiding a call to Runner#grepTotal(), which is incompatible with parallel mode). This is within spec, so should not be a breaking change.
  • add lib/buffered-runner.js (BufferedRunner); subclass of Runner which overrides the run() method.
    • There's a little custom finite state machine in here. didn't want to pull in a third-party module, but we should consider doing so if we use FSM's elsewhere.
    • when DEBUG=mocha:parallel* is in the env, this module will output statistics about the worker pool every 5s
    • the run() method looks a little weird because I wanted to use async/await, but the method it is overriding (Runner#run) is not async
    • traps SIGINT to gracefully terminate the pool
    • pulls in promise.allsettled polyfill to handle workers that may have rejected with uncaught exceptions
    • "bail" support is best-effort.
    • the ABORTING state is only for interruption via SIGINT or if allowUncaught is true and we get an uncaught exception
  • Hook, Suite, Test: add a serialize() method. This pulls out the most relevant information about the object for transmission over IPC. It's called by worker processes for each event received by its Runner; event arguments (e.g., test or suite) are serialized in this manner. Note that this limits what reporters have access to, which may break compatibility with third-party reporters that may use information that is missing from the serialized object. As those cases arise, we can add more information to the serialized objects (in some cases). The $$ convention tells the deserializer to turn the property into a function which returns the passed value, e.g., test.fullTitle().
  • lib/mocha.js:
    • refactor Mocha#reporter for nicer parameter & variable names
    • rename loadAsync to lazyLoadFiles, which is more descriptive, IMO. It's a private property, so should not be a breaking change.
    • Constructor will dynamically choose the appropriate Runner
  • lib/runner.js: BufferedRunner needs the options from Mocha#options, so I updated the parent method to define the parameter. It is unused here.
  • add lib/serializer.js: on the worker process side, manages event queue serialization; manages deserialization of the event queue in the main process.
    • I spent a long time trying to get this working. We need to account for things like Error instances, with their stack traces, since those can be event arguments (e.g., EVENT_TEST_FAIL sends both a Test and the Error). It's impossible to serialize circular (self-referential) objects, so we need to account for those as well.
    • Not super happy with the deserialization algorithm, since it's recursive, but it shouldn't be too much of an issue because the serializer won't output circular structures.
    • Attempted to avoid prototype pollution issues
    • Much of this works by mutating objects, mostly because it can be more performant. The code can be changed to be "more immutable", as that's likely to be easier to understand, if it doesn't impact performance too much. We're serializing potentially very large arrays of stuff.
    • The __type prop is a hint for the deserializer. This convention allows us to re-expand plain objects back into Error instances, for example. You can't send an Error instance over IPC!
  • add lib/worker.js:
    • registers its run() function with workerpool to be called by main process
    • if DEBUG=mocha:parallel* is set, will output information (on an interval) about long-running test files
    • afaik the only way run() can reject is if allowUncaught is true or serialization fails
    • any user-supplied reporter value is replaced with the Buffered reporter. thus, reporters are not validated.
    • the worker uses Runner, like usual.
  • tests:
    • see test/integration/options/parallel.spec.js for the interesting stuff
    • upgrade unexpected for "to have readonly property" assertion
    • upgrade unexpected-eventemitter for support async function support
    • integration test helpers allow Mocha's developers to use --bail and --parallel, but will default to --no-bail and --no-parallel.
  • etc:
    • update .eslintrc.yml for new Node-only files
    • increase default timeout to 1000 (also seen in another PR) and use parallel mode by default in .mocharc.yml
    • run node unit tests in serial as sort of a smoke test, as otherwise all our tests would be run in parallel
    • karma, browserify: ignore files for parallel support
    • force color output in CI. this is nice on travis, but ugly on appveyor. either way, it's easier to read than having no color

Ref: #4198

@boneskull boneskull added type: feature enhancement proposal semver-minor implementation requires increase of "minor" version number; "features" area: node.js command-line-or-Node.js-specific labels Apr 21, 2020
@boneskull boneskull self-assigned this Apr 21, 2020
@boneskull
Copy link
Member Author

In general, I believe this to be a semver minor change, but it might want to land in v8.0.0 anyway.

@boneskull
Copy link
Member Author

updated package-lock.json

@boneskull
Copy link
Member Author

This needs changes from #4240 and #4244 in order for the build to pass.

@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch from bb1b327 to ac25cf7 Compare May 5, 2020 18:44
@boneskull
Copy link
Member Author

--grep doesn't work because the value winds up being a RegExp, and you can't express such a thing in JSON, so I'm going to need to add something that serializes/deserializes options, or we coerce the value to a regexp just-in-time.

will also need to see if there are other non-primitive-non-array options that might run into trouble

also, I don't think exclusive tests can work. we can certainly skip tests, but since we aren't loading all files up-front, we won't necessarily be able to tell if there's an only somewhere until after we've already run other tests...

@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch from ac25cf7 to 331fca9 Compare May 6, 2020 23:52
@coveralls
Copy link

coveralls commented May 7, 2020

Coverage Status

Coverage increased (+0.2%) to 93.585% when pulling 2ed47ef on boneskull/issue/2839-parallel into 6d60eb0 on master.

@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch 2 times, most recently from 9ab7f71 to cfbc32e Compare May 12, 2020 17:46
@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch from cfbc32e to b919cac Compare May 19, 2020 23:07
@boneskull boneskull marked this pull request as ready for review May 19, 2020 23:07
@boneskull boneskull linked an issue May 19, 2020 that may be closed by this pull request
@boneskull
Copy link
Member Author

#4244 and #4240 have both been merged, and this has been rebased onto master. It's ready for review, assuming CI passes.

karma.conf.js Outdated
.ignore('./lib/reporters/buffered.js')
.ignore('./lib/serializer.js')
.ignore('./lib/worker.js')
.ignore('./lib/pool.js')
Copy link
Member

Choose a reason for hiding this comment

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

workerpool looks browser compatible enough to support our browser range. Is there a way to actually get these files in the browser bundle and get parallel runs in there as well?

I'm not super happy with keeping this blacklist for browser builds. It would be better if we could make a split where they are actually separate dependency graphs to make it less magic. I've already started doing some of that on the rollup branch, so I might just follow up on that thread there or later after both of those features are merged

Copy link
Member Author

Choose a reason for hiding this comment

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

workerpool looks browser compatible enough to support our browser range. Is there a way to actually get these files in the browser bundle and get parallel runs in there as well?

yes, it might work. I didn't want to try it in this PR though.

Copy link
Contributor

@nicojs nicojs left a comment

Choose a reason for hiding this comment

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

I've taken a look. You nailed the serialization stuff, couldn't really find anything wrong with it.

I've got quite a few remarks, but some of them are questions and others are opinionated. Please tell me if you disagree with them.

lib/mocha.js Outdated Show resolved Hide resolved
* This `Runner` delegates tests runs to worker threads. Does not execute any
* {@link Runnable}s by itself!
*/
class BufferedRunner extends Runner {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it called a "Buffered" runner? Since the description isn't explaining that either. The fact that it uses a buffer internally doesn't help the consumer of this class.

Suggestion: ParallelRunner.

Also: we're allowed to use es classes? Awesome! 😍

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is it called a "Buffered" runner? Since the description isn't explaining that either. The fact that it uses a buffer internally doesn't help the consumer of this class.

BufferedParallelRunner could be more accurate. The idea is that ParallelRunner is fine until we add an unbuffered parallel runner; e.g., a streaming parallel runner. This would be especially useful for streaming machine-readable results; not so much for human-readable output. It would more closely mimic serial mode; how reporters generate output in real-time. But it would also be confusing to use with a structured reporter like spec, because it'd just be a firehose with random indentation. Would work for dot, though.

Also: we're allowed to use es classes? Awesome! 😍

This is fair game in bin/, scripts/, lib/nodejs/, lib/cli/ and test/node-unit... essentially, code that only runs in Node.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

BufferedParallelRunner could be more accurate. The idea is that ParallelRunner is fine until we add an unbuffered parallel runner; e.g., a streaming parallel runner.

I would expect the streaming might be a setting of the ParellelRunner in the future. It is a minor implementation detail, right? Just a streaming reporter configured in the worker process, rather than a buffered reporter.

lib/buffered-runner.js Outdated Show resolved Hide resolved
lib/pool.js Outdated Show resolved Hide resolved
lib/pool.js Outdated Show resolved Hide resolved
lib/cli/run.js Outdated Show resolved Hide resolved
@@ -557,6 +557,21 @@ Suite.prototype.cleanReferences = function cleanReferences() {
}
};

/**
* Returns an object suitable for IPC.
* Functions are represented by keys beginning with `$$`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the need for $$? We're using 2 'magic' serializer markers. __type and $$-prefixed values. Maybe we can align those to be the same. I would also like the purpose to be directly clear for readers, maybe prefix with: __serialized__?

Copy link
Member Author

Choose a reason for hiding this comment

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

these magic keys do have two different meanings, but I agree, I can be much more explicit about their meaning.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am going to defer this task until later. I think there are more refactors that can happen here, but my goal is to get this merged first. not too concerned about regressions

test/node-unit/serializer.spec.js Show resolved Hide resolved
lib/serializer.js Outdated Show resolved Hide resolved
test/node-unit/worker.spec.js Show resolved Hide resolved
@nicojs
Copy link
Contributor

nicojs commented May 22, 2020

Do you feel it's part of an MVP? Or can this land & be released without it?

Unsure if a joke 😏. I don't think anyone could argue that progress reporting is a must-have for MVP 😉. People can opt-out of --parallel mode if they feel strongly about it.

@@ -95,6 +96,9 @@ jobs:
script: true
name: 'Prime cache'

env:
- 'NODE_OPTIONS="--trace-warnings"'
Copy link
Member Author

Choose a reason for hiding this comment

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

imo this is useful just to leave in.

}

Copy link
Member Author

Choose a reason for hiding this comment

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

my assumption is that people tend not to love nested ternaries. but if someone wants it, I will abide

@boneskull
Copy link
Member Author

boneskull commented May 22, 2020

@nicojs Was thinking about the lazyLoadFiles thing, and the variable is not really needed. What's going on is that, by default, Mocha#run will implicitly call Mocha.loadFiles unless this variable is set. Perhaps it's better to load files explicitly; rather than:

mocha.addFile('my-file.js');
mocha.run()

do:

mocha.addFile('my-file.js');
mocha.loadFiles();
mocha.run();

For the parallel/ESM case, it's currently:

mocha.addFile('my-file.js');
mocha.lazyLoadFiles = false;
await mocha.loadFilesAsync();
mocha.run();

but it would then eliminate the need to set the variable:

mocha.addFile('my-file.js');
await mocha.loadFilesAsync();
mocha.run();

This would be a breaking change, however, and so might not be worth doing. But it also allows us to add a state between INIT and RUNNING, e.g., READY, which is set to true once one of Mocha#loadFiles or Mocha#loadFilesAsync has successfully completed. It complicates the state machine, so I didn't want to do it... but figure it's worth mentioning.

) {
return this;
}
if (this._state !== mochaStates.INIT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: the state of the mocha instance is reset to INIT after a run (if cleanReferencesAfterRun is false). So a user can change the parallelMode after mocha has run.

This is fine, as long as we implement the dispose on the bufferedRunner correctly. It would mean that the parallel runner / buffered runner is disposed and a new one is started. We might want to change this behavior in the future, but it's fine for now I would say.

test/node-unit/worker.spec.js Show resolved Hide resolved
@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch 3 times, most recently from 986f412 to c694410 Compare May 27, 2020 18:20
@boneskull
Copy link
Member Author

appveyor is acting very strangely; I'm not sure why a worker thread is getting invoked in the "watch" tests. I've squashed this again

@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch 3 times, most recently from 3f58954 to 155841a Compare May 27, 2020 19:08
@boneskull boneskull requested a review from nicojs May 27, 2020 20:33
@boneskull
Copy link
Member Author

@nicojs I think I've addressed your concerns here, but at this point I'm unsure because I'm not sure where to find your review.

I currently can't view the Netlify deploy to see what's wrong there, but, the docs build on my machine. 😄

@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch from fd92e62 to f854f51 Compare May 27, 2020 21:26
@nicojs
Copy link
Contributor

nicojs commented May 28, 2020

Thanks @boneskull . I will try to get to this today. Might be tomorrow.

Copy link
Contributor

@nicojs nicojs left a comment

Choose a reason for hiding this comment

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

@boneskull I think this can land as is. I would prefer to get the name change in (BufferedRunner -> ParallelBufferedRunner), but otherwise this looks good.

* add support for running tests in parallel mode

> (this PR depends on most other PRs linked to #4198, so they should be merged first; documentation will be in another PR)

This PR adds support for running test files in parallel via `--parallel`.  For many cases, this should "just work."

When the `--parallel` flag is supplied, Mocha will swap out the default `Runner` (`lib/nodejs/runner.js`) for `ParallelBufferedRunner` (`lib/nodejs/parallel-buffered-runner.js`).

`ParallelBufferedRunner` _extends_ `Runner`.  `ParallelBufferedRunner#run()` is the main point of extension.  Instead of executing the tests in serial, it will create a pool of worker processes (not worker _threads_) based on the maximum job count (`--jobs`; defaults to `<number of CPU cores> - 1`).  Both `ParallelBufferedRunner` and the `worker` module consume the abstraction layer, [workerpool](https://npm.im/workerpool).

`ParallelBufferedRunner#run()` does _not_ load the test files, unlike `Runner#run()`.  Instead, it has a list of test files, and puts these into an async queue.  The `EVENT_RUN_BEGIN` event is then emitted.  As files enter the queue, `ParallelBufferedRunner#run()` tells `workerpool` to execute the `run()` function of the pool.  `workerpool` then launches as many worker processes are needed--up to the maximum--and executes the `run()` function with a single filepath and any options for a `Mocha` instance.

The first time `lib/nodejs/worker.js` is invoked, it will "bootstrap" itself, by handling `--require`'d modules and validating the UI.  Note that _reporter validation_ does not occur.  Once bootstrapped, it instantiate `Mocha`, add the single file, swap any reporter out for the `ParallelBuffered` reporter (`lib/nodejs/reporters/parallel-buffered.js`) then execute `Mocha#run()`, which invokes `Runner#run()`.

The `ParallelBuffered` reporter listens for events emitting from the `Runner` instance, like a reporter usually does.  But instead of outputting to the console, it buffers the events in a queue.  Once the file has completed running, the queue is drained: the events collected are (trivially) serialized for transmission back to the main process.  `ParallelBufferedRunner#run()` receives the list of events, trivially _deserializes_ them, and re-emits the events to whatever the chosen reporter is (e.g., the `spec` reporter).  In this way, the reporters don't know that the tests were run in parallel.  Practically, the user will see reporter output in "chunks" instead of the "stream" of results they usually expect.  This method ensures that while the test files run in a nondeterministic order, the reporter output will be deterministic for any given test file.

Once the result (the queue of events) has been returned to the main process, the worker process stays open, but waits for further instruction.  If there are more files in `ParallelBufferedRunner#run()`'s queue, `workerpool` will instruct the worker to take the next file from the list, and so on, and so forth.  When all files have executed, the pool terminates, the `EVENT_RUN_END` event is emitted, and the reporter handles it.

Note that exclusive tests ("only") cannot work in parallel mode, because we do not load all files up-front.

> (this section is pasted from the documentation with minimal edits)

### Caveats: Reporters

Due to the nature of the following reporters, they cannot work when running tests in parallel:

- `markdown`
- `progress`
- `json-stream`

These reporters expect Mocha to know _how many tests it plans to run_ before execution. This information is unavailable in parallel mode, as test files are loaded only when they are about to be run.

### Caveats: Buffered Output

In serial mode, tests results will "stream" as they occur. In parallel mode, reporter output is _buffered_; reporting will occur after each file is completed. In practice, the reporter output will appear in "chunks" (but will otherwise be identical).

### Caveats: Nondeterminism

In parallel mode, we have no guarantees about the order in which test files will be run--or what process runs them--as it depends on the execution times of the test files.

Because of this, the following options _cannot be used_ in parallel mode:

- `--file`
- `--sort`
- `--delay`

Because running tests in parallel mode uses more system resources at once, the OS may take extra time to schedule and complete some operations. For this reason, test timeouts may need to be increased either globally or otherwise.

### Caveats: Other Impacted Options

When used with `--bail` (or `this.bail()`) to exit after the first failure, it's likely other tests will be running at the same time. Mocha must shut down its worker processes before exiting.

Likewise, subprocesses may throw uncaught exceptions. When used with `--allow-uncaught`, Mocha will "bubble" this exception to the main process, but still must shut down its processes.

`--forbid-only` does not work in parallel mode, for a similar reason to the unsupported reporters.

> _NOTE: This only applies to test files run parallel mode_.

### Caveats: Root Hooks

A _root hook_ is a hook in a test file which is _not defined_ within a suite. An example using the `bdd` interface:

```js
// test/setup.js
beforeEach(function() {
  doMySetup();
});

afterEach(function() {
  doMyTeardown();
});
```

When run (in the default "serial" mode) via `mocha --file "./test/setup.js" "./test/**/*.spec.js"`, `setup.js` will be executed _first_, and install the two hooks shown above for every test found in `./test/**/*.spec.js`.

**When Mocha runs in parallel mode, test files do not share the same process.** Consequently, a root hook defined in test file _A_ won't be present in test file _B_.

There are a (minimum of) two workarounds for this:

1. `require('./setup.js')` or `import './setup.js'` at the top of every test file. Best avoided for those averse to boilerplate.
1. _Recommended_: Define root-level hooks in a required file, using the new (also as of VERSION) Root Hook Plugin system.

### Caveats: Node.js Only, For Now

Parallel mode is only available in Node.js.

### Migration Checklist

If you find your tests don't work properly when run with `--parallel`, either shrug and move on, or use this handy-dandy checklist to get things working:

- ✅ Ensure you are using a supported reporter.
- ✅ Ensure you are not using other unsupported flags.
- ✅ Double-check your config file; options set in config files will be merged with any command-line option.
- ✅ Look for root-level hooks in your tests. Move them into a root hook plugin.
- ✅ Do any assertion, mock, or other test libraries you're consuming use root hooks? They may need to be migrated for compatibility with parallel mode.
- ✅ If tests are unexpectedly timing out, you may need to increase the default test timeout (via `--timeout`)
- ✅ Ensure your tests do not depend on being run in a specific order.
- ✅ Ensure your tests clean up after themselves; remove temp files, handles, sockets, etc. Don't try to share state or resources between test files.

### About Parallelism

Some types of tests are _not_ so well-suited to run in parallel. For example, extremely timing-sensitive tests, or tests which make I/O requests to a limited pool of resources (such as opening ports, or automating browser windows, hitting a test DB, or remote server, etc.).

Free-tier cloud CI services may not provide a suitable multi-core container or VM for their build agents. Regarding expected performance gains in CI: your mileage may vary. It may help to use a conditional in a `.mocharc.js` to check for `process.env.CI`, and adjust the job count as appropriate.

It's unlikely (but not impossible) to see a performance gain from a job count _greater than_ the number of available CPU cores. That said, _play around with the job count_--there's no one-size-fits all, and the unique characteristics of your tests will determine the optimal number of jobs; it may even be that fewer is faster!

### Change Details

- updated signal handling in `bin/mocha` to a) better work with Windows, and b) work properly with `--parallel` to avoid leaving zombie workers
- docstrings in `lib/nodejs/cli/collect-files.js`
- refactors in `lib/nodejs/cli/run-helpers.js` and `lib/nodejs/cli/watch-run.js`.  We now have four methods:
    - `watchRun()` - serial + watch
    - `singleRun()` - serial
    - `parallelWatchRun()` - parallel + watch
    - `parallelRun()` - parallel
- `lib/nodejs/cli/run.js` and `lib/nodejs/cli/run-option-metadata.js`: additions for new options and checks for incompatibility
- add `lib/nodejs/reporters/buffered.js` (`ParallelBuffered`); this reporter is _not_ re-exported in `Mocha.reporters`, since it should only be invoked internally.
- tweak `landing` reporter to avoid referencing `Runner#total`, which is incompatible with parallel mode.  It didn't need to do so in the first place!
- the `tap` reporter now outputs the plan at the _end_ instead of at the beginning (avoiding a call to `Runner#grepTotal()`, which is incompatible with parallel mode).  This is within spec, so should not be a breaking change.
- add `lib/nodejs/parallel-buffered-runner.js` (`ParallelBufferedRunner`); subclass of `Runner` which overrides the `run()` method.
    - There's a little custom finite state machine in here.  didn't want to pull in a third-party module, but we should consider doing so if we use FSM's elsewhere.
    - when `DEBUG=mocha:parallel*` is in the env, this module will output statistics about the worker pool every 5s
    - the `run()` method looks a little weird because I wanted to use `async/await`, but the method it is overriding (`Runner#run`) is _not_ `async`
    - traps `SIGINT` to gracefully terminate the pool
    - pulls in [promise.allsettled](https://npm.im/promise.allsettled) polyfill to handle workers that may have rejected with uncaught exceptions
    - "bail" support is best-effort.
    - the `ABORTING` state is only for interruption via `SIGINT` or if `allowUncaught` is true and we get an uncaught exception
- `Hook`, `Suite`, `Test`: add a `serialize()` method.  This pulls out the most relevant information about the object for transmission over IPC.  It's called by worker processes for each event received by its `Runner`; event arguments (e.g., `test` or `suite`) are serialized in this manner.  Note that this _limits what reporters have access to_, which may break compatibility with third-party reporters that may use information that is missing from the serialized object.  As those cases arise, we can add more information to the serialized objects (in some cases).  The `$$` convention tells the _deserializer_ to turn the property into a function which returns the passed value, e.g., `test.fullTitle()`.
- `lib/nodejs/mocha.js`:
    - refactor `Mocha#reporter` for nicer parameter & variable names
    - rename `loadAsync` to `lazyLoadFiles`, which is more descriptive, IMO.  It's a private property, so should not be a breaking change.
    - Constructor will dynamically choose the appropriate `Runner`
- `lib/nodejs/runner.js`: `ParallelBufferedRunner` needs the options from `Mocha#options`, so I updated the parent method to define the parameter.  It is unused here.
- add `lib/nodejs/serializer.js`: on the worker process side, manages event queue serialization; manages deserialization of the event queue in the main process.
    - I spent a long time trying to get this working.  We need to account for things like `Error` instances, with their stack traces, since those can be event arguments (e.g., `EVENT_TEST_FAIL` sends both a `Test` and the `Error`).  It's impossible to serialize circular (self-referential) objects, so we need to account for those as well.
    - Not super happy with the deserialization algorithm, since it's recursive, but it shouldn't be too much of an issue because the serializer won't output circular structures.
    - Attempted to avoid prototype pollution issues
    - Much of this works by mutating objects, mostly because it can be more performant.  The code can be changed to be "more immutable", as that's likely to be easier to understand, if it doesn't impact performance too much.  We're serializing potentially very large arrays of stuff.
    - The `__type` prop is a hint for the deserializer.  This convention allows us to re-expand plain objects back into `Error` instances, for example.  You can't send an `Error` instance over IPC!
- add `lib/nodejs/worker.js`:
    - registers its `run()` function with `workerpool` to be called by main process
    - if `DEBUG=mocha:parallel*` is set, will output information (on an interval) about long-running test files
    - afaik the only way `run()` can reject is if `allowUncaught` is true or serialization fails
    - any user-supplied `reporter` value is replaced with the `ParallelBuffered` reporter.  thus, reporters are not validated.
    - the worker uses `Runner`, like usual.
- tests:
    - see `test/integration/options/parallel.spec.js` for the interesting stuff
    - upgrade `unexpected` for "to have readonly property" assertion
    - upgrade `unexpected-eventemitter` for support async function support
    - integration test helpers allow Mocha's developers to use `--bail` and `--parallel`, but will default to `--no-bail` and `--no-parallel`.
    - split some node-specific tests out of `test/unit/mocha.spec.js` into `test/node-unit/mocha.spec.js`
    - add some missing coverage in `test/node-unit/worker.spec.js`
- etc:
    - update `.eslintrc.yml` for new Node-only files
    - increase default timeout to `1000` (also seen in another PR) and use `parallel` mode by default in `.mocharc.yml`
    - run node unit tests _in serial_ as sort of a smoke test, as otherwise all our tests would be run in parallel
    - karma, browserify: ignore files for parallel support
    - force color output in CI. this is nice on travis, but ugly on appveyor.  either way, it's easier to read than having no color
    - move non-CLI-related node-specific files into `lib/nodejs/nodejs/`
    - correct some issues with APIs not marked `@private`
    - add some istanbul directives to ignore some debug statements
    - add `utils.isBrowser()` for easier mocking of a `process.browser === true` situation
    - add `createForbiddenExclusivityError()`

Ref: #4198

Signed-off-by: Christopher Hiller <boneskull@boneskull.com>
@boneskull boneskull force-pushed the boneskull/issue/2839-parallel branch from afc00ce to 2ed47ef Compare June 1, 2020 19:57
@boneskull boneskull merged commit 2078c32 into master Jun 1, 2020
@boneskull boneskull deleted the boneskull/issue/2839-parallel branch June 1, 2020 21:19
@boneskull boneskull modified the milestones: v8.0.0, next Jun 1, 2020
@kyrylodolynskyi
Copy link

When do you plan to publish this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: node.js command-line-or-Node.js-specific semver-minor implementation requires increase of "minor" version number; "features" type: feature enhancement proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

run Mocha's tests concurrently
5 participants