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

adding a MOCHA_WORKER_ID environment variable, unique for each worker #4463

Closed
wants to merge 2 commits into from

Conversation

catdad
Copy link

@catdad catdad commented Oct 3, 2020

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions.

Description of the Change

This change addresses #4456. It adds a MOCHA_WORKER_ID environment variable that is unique for each worker, so that it may be used for by any test resources that need to use global resources (network ports, filesystem paths, etc.) but ensure that these are unique in order to avoid conflicts during parallel tests.

Alternate Designs

I considered using forkArgs instead of an environment variable, so that an ID may be accessed using process.argv[2]. I did not want to do this in case adding arguments was necessary for internal uses in the future.

Why should this be in core?

It is common for tests to use global resources like network ports, which can conflict during parallel testing. Without a feature like this in core, users would be left on their own to create a home-brewed solution (using filesystem files as signals, sockets that elect a leader in the cluster so that one process hands out all global data, a standalone cluster orchestration system), all of which add a lot of complication to writing tests. This would also be a difference when writing tests that run in series vs. parallel, as this concern would not exist (and potentially needs to be turned off) for test in series, and would add pain points when taking existing serial tests and having them run in parallel. This makes using mocha significantly less fun.

This feature takes that pain point away. For example, allocating ports would once again be trivial, using const port = 1234 + Number(process.env.MOCHA_WORKER_ID || 0).

Other parallel testing frameworks, like jest, already use this approach: jestjs/jest#2284

Benefits

This solves common pain points for developers writing tests.

Possible Drawbacks

Other than more complexity, I can't think of a drawback for the feature itself.

Since workerpool does not itself provide an option for worker-specific options or to execute code deterministically in a specific worker (only an option to add to the queue to execute on any available worker), I had to use a workaround in the implementation. This workaround could make internal updates more difficult and adds extra requirements to replacing workerpool if a need arises to use a different implementation internally. I personally believe that this tradeoff is worth it, though I am probably biased here.

Applicable issues

@catdad
Copy link
Author

catdad commented Oct 3, 2020

I submitted this as a draft for now because I have not gotten a chance to write tests yet.

I am also interested in getting some feedback as early as possible in the PR process. Since workerpool does not have the option to add per-worker options, or even directly execute code on a specific worker, I had to get a little bit creative in the implementation.

@boneskull
Copy link
Contributor

Have you tried using the PID, e.g.:

const port = 1234 + Number(process.pid || 0)

@boneskull boneskull added type: feature enhancement proposal semver-minor implementation requires increase of "minor" version number; "features" labels Oct 6, 2020
@catdad
Copy link
Author

catdad commented Oct 6, 2020

The randomness of PIDs makes this undesirable to me. A PID can cause a conflict with an existing port being in use on the system, and this will not be consistent across test runs. I can run the tests successfully once, then get a conflict the second time, then run them successfully on the third, etc. A solution for this should be deterministic and repeatable (i.e. a parallel test run with 3 jobs will always get the numbers [1, 2, 3] every time, albeit not necessarily the same number for the same exact test each time).

@forty
Copy link
Contributor

forty commented May 20, 2021

For reference another use case where deterministic IDs would be desirable #4456 (comment)

@juergba
Copy link
Contributor

juergba commented Jan 22, 2022

superseded by #4813

@juergba juergba closed this Jan 22, 2022
@juergba juergba added the area: parallel mode Regarding parallel mode label Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: parallel mode Regarding parallel mode 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.

4 participants