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 parallel mode support #331

Merged
merged 1 commit into from
Nov 1, 2020

Conversation

kolbasik
Copy link
Contributor

Resolve #321

Purposes:

  • to add support of parallel mode by listening the events from mocha workers

PS: mocha uses a custom test serializer so I have extended their serializer to consume the context as well

@sshaar08
Copy link

@kolbasik can I try this locally? I'm assuming I just use the branch in my package.json instead of version right?

@kolbasik
Copy link
Contributor Author

kolbasik commented Oct 27, 2020

Hi @sshaar08, I could not find a build package on npmjs.com
Apparently, it's a manual process to release a new version.

PS: if you just want to try locally then you could grab addContext.js and mochawesome.js from this PR and replace those files in ./node_modules/mochawesome/src folder

@kolbasik
Copy link
Contributor Author

kolbasik commented Oct 27, 2020

Meantime until this PR is here. I am using the following patch in my project:

https://gist.github.com/kolbasik/da92e9173bd262ac889cc7ed1d8e1b2e

In order to use it you need to define it as custom reporter instead of original mochawsome.

mocha --reporter ./mochawesome.js --parallel 

Plus, if you use addContext then you also need to import this file in hook or explicitly in the tests

@sshaar08
Copy link

@adamgruber Not sure if you have time but would be great to get this in.

@adamgruber
Copy link
Owner

@kolbasik Thanks for the PR! I'd love for this reporter to be able to support parallel mode.

I was doing some local testing with your branch and I'm seeing some inconsistencies with the reporter output between a normal test and a parallel test. These include:

  • Filenames are missing in parallel run
  • Total tests count is different
  • "Skipped" count is different
  • Overall durations are different. (I don't expect these to be identical but in my tests there's a decent difference, with the parallel tests having a much longer total duration)

To reproduce my local testing, update the npm scripts inside package.json to include:

"test:fn": "mocha test-functional/ --config test-functional/.mocharc.json",
"test:par": "mocha --parallel test-functional/ --config test-functional/.mocharc.json",

I think it's important that the results be the same whether run normally or in parallel. Or at least have good documentation on what differences to expect and why.

@kolbasik
Copy link
Contributor Author

kolbasik commented Oct 28, 2020

Hi @adamgruber , I have made changes according to your points.

Filenames are missing in parallel run

I have extended Suite, Hook and Test to pass additional data from mocha worker to fix this case.
It requires to register mochawesome as a hook to work correctly in parallel mode. This is reflected in the README.

$ mocha --reporter mochawesome --require mochawesome/register --parallel ./tests
- Total tests count is different
- "Skipped" count is different

Mocha does not provide information about skipped tests in parallel mode. Therefore, the skipped tests is always 0, and the total is less by the skipped value. I have mentioned about it in README as well.

Overall durations are different. (I don't expect these to be identical but in my tests there's a decent difference,
with the parallel tests having a much longer total duration)

Spawning worker processes is not free. So you should not use parallel option if tests are fast. Jest has the same behavior, for small amount of tests you need to use --runInBand option.

PS: I have added test:par to the scripts to see an example of the advantages of using the parallel option:

npm run test:par
npm run test:par -- --parallel

@adamgruber adamgruber merged commit 4afd90b into adamgruber:master Nov 1, 2020
@kolbasik
Copy link
Contributor Author

kolbasik commented Nov 1, 2020

Thanks, @adamgruber

@kolbasik
Copy link
Contributor Author

kolbasik commented Nov 1, 2020

Hi @adamgruber, sorry for disturbance. You have released mochawesome 6.2.0 version and it should contain register.js file in the root of package: https://unpkg.com/browse/mochawesome@6.2.0/
Otherwise --require mochawesome/register would not work.
Currently works --require mochawesome/src/register

PS: I have created another PR to fix it #332

@juergba
Copy link

juergba commented Aug 24, 2021

@kolbasik I know that Mocha's reporting in parallel mode is rather ... well ... suboptimal.

Mocha does not provide information about skipped tests in parallel mode.

This is clearly a bug. Can you tell me, which additional properties you need, please? I'm in doubt about ctx though, we have to keep the serialized data transmitted via IPC as small as possible.

@kolbasik
Copy link
Contributor Author

Hi @juergba, sorry for the delay, I was on vacation.

Shortly, it is not about missing properties. It's more about Mochawsome feature.

Mocha just has 3 states for the tests

  • PASS if passed
  • FAIL if failed
  • PENDING if used it.skip or xit

And you could see that only those 3 states are handled by Mocha everywhere, listening to EVENT_TEST_PASS, EVENT_TEST_FAIL and EVENT_TEST_PENDING evetns

Mochawsome has the same states plus SKIPPED if the tests were not handled due to the exceptions in the hooks before.

mochawesome/src/utils.js

Lines 185 to 186 in b600095

cleaned.skipped =
!cleaned.pass && !cleaned.fail && !cleaned.pending && !cleaned.isHook;

In order to do that Mochawesome also tracks 2 more fields: total registered and total skipped.
https://github.com/adamgruber/mochawesome/blob/master/src/mochawesome.js#L24-L27

  • registered is the total amount of tests to be executed
  • skipped is the tests which were not touched by Mocha runner

So, here we could have misunderstanding between PENDING and SKIPPED. When devs use it.skip or xit then Mocha marked them as PANDING not SKIPPED. And later Mochawsome marks all unhandled tests as SKIPPED because of the following:
https://github.com/mochajs/mocha/blob/bbf0c11b29544de91a18c1bd667c975ee44b7c90/lib/runner.js#L801

mochawesome/src/utils.js

Lines 185 to 186 in b600095

cleaned.skipped =
!cleaned.pass && !cleaned.fail && !cleaned.pending && !cleaned.isHook;

Mochawesome is able to calculate the amount of SKIPPED tests only in sync mode since it has an access to the original list of Mocha's suites: https://github.com/adamgruber/mochawesome/blob/master/src/mochawesome.js#L207-L211
In the parallel mode Mochawesome has to restore the list of suites by events. Using EVENT_TEST_PASS, EVENT_TEST_FAIL and EVENT_TEST_PENDING events is not possible to restore the orginal suites from workers.

To reproduce above scenario and to get more details you could do the following:

  1. Replace it with xit or it.skip in the ./test-parallel/test-1.js file
  2. Add below to the ./test-parallel/test-2.js file
beforeEach(() =>{
    throw new Error("Skipped Test")
})
  1. Run tests in the sync and parallel mode
yarn mocha test-parallel --config test-parallel/.mocharc.json
yarn mocha test-parallel --config test-parallel/.mocharc.json  --parallel
  1. compare sync and parallel reports.

Therefore, Mocha does not send events about existing tests of workers if any errors occurred before the execution of that tests.
If the Mocha runner is extended with additional events to be able to restore the original suites tree then we could also add this feature to the Mochawsome report.

Currently, Mochawesome resports shows the same information as default Mocha reports in the parallel mode.
I am not quite sure how this is important to change the Mocha runner since it's only Mochawsome report feature.

I hope that I have answered on your question.

@juergba
Copy link

juergba commented Sep 8, 2021

@kolbasik thank you for your reply.

Yes, I'm not happy either with Mocha's hook pattern. A failing hook drops its corresponding tests, they just disappear without any reporting. Btw besides the existing test states (PASS, FAIL, PENDING (skipped)) and the skipped-by-hook-failure: maybe DROPPED (you name it SKIPPED), there is one more state missing: undefined for tests which have never been touched by the runner, eg. due to not matching grep or .only.

I have extended Suite, Hook and Test to pass additional data from mocha worker to fix this case.

If you extend the SUITE END with the tests array, then you should have enough information to calculate your SKIPPED tests, I guess.

@kolbasik
Copy link
Contributor Author

Hi @jurgen, thank you for your suggestion regarding the SUITE END. I tried to include all the tests in the suites and listen to EVENT_SUITE_END. Later I discovered a case with missing subsuites and not only tests for the same reason due to the failed hooks.
So, I have made a full dump of the root suite and listent to it in the EVENT_SUITE_END. It allows to build a correct report.

@juergba
Copy link

juergba commented Oct 7, 2021

@kolbasik thanks.
What about the speed loss by the serialization of the complete root suite, is there any advantage left between serial <=> parallel mode?

@jurgen
Copy link

jurgen commented Oct 7, 2021 via email

@kolbasik
Copy link
Contributor Author

kolbasik commented Oct 7, 2021

What about the speed loss by the serialization of the complete root suite, is there any advantage left between serial <=> parallel mode?

@juergba , no matter how much time is the overhead of running tests in parallel.
The parallel mode always would helps if you have really long running tests. Seriallization and transfering data is really fast at my machine (16 CPU, 32 GB memory, SSD).
For example, let assume that if parallel overhead takes 500ms per file and you have 5 tests with 3 secons of execution then

  • in sync mode it would take 5 tests * 3 seconds = 15 seconds
  • in parallel mode it would take 5 tests * (3 + 0.5 seconds) / 5 threads = 3.5 seconds in total, which is less compared to sync

wll8 added a commit to wll8/mockm that referenced this pull request Apr 24, 2022
- MOCKM_REGISTRY 环境变量用于保证稳定的自动安装依赖
- --require 用于处理并行测试时不能正确显示 html
  - adamgruber/mochawesome#331
wll8 added a commit to wll8/mockm that referenced this pull request Nov 18, 2022
- MOCKM_REGISTRY 环境变量用于保证稳定的自动安装依赖
- --require 用于处理并行测试时不能正确显示 html
  - adamgruber/mochawesome#331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

report showing no test information when running mocha in parallel mode
5 participants