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

perf(mocha): run tests in parallel #4374

Merged
merged 3 commits into from
Jul 17, 2020
Merged

Conversation

curbengh
Copy link
Contributor

@curbengh curbengh commented Jun 23, 2020

What does it do?

A new feature of mocha@8. Consistently 3s faster in my single-core VM.
nyc (for now, v15.1.0) doesn't support mocha's parallel mode yet, so parallel mode is disabled in test coverage job.

How to test

git clone -b parallel-mocha https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test

Screenshots

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

@curbengh
Copy link
Contributor Author

curbengh commented Jun 23, 2020

it's not reliable yet, unit test sometimes fail with:

  1) Hexo
       Console
         generate
           update theme source files:

      AssertionError: expected 'b' to deeply equal 'bb'
      + expected - actual

      -b
      +bb
      
      at Context.<anonymous> (hexo/test/scripts/console/generate.js:207:22)

@SukkaW
Copy link
Member

SukkaW commented Jun 23, 2020

it's not reliable yet, unit test sometimes fail with:

Appears that failed tests are related with fs operation.

@curbengh
Copy link
Contributor Author

nyc is currently incompatible with mocha parallel mode, can confirm in my machine.

@SukkaW
Copy link
Member

SukkaW commented Jul 2, 2020

@curbengh We can run coverage only once, while the other tests can be still run in parallel.

@curbengh
Copy link
Contributor Author

curbengh commented Jul 10, 2020

We can run coverage only once, while the other tests can be still run in parallel.

Mocha supports environment variable MOCHA_PARALLEL=0. It should be possible to disable it in coverall job.


Edit: Apparently not...

@curbengh
Copy link
Contributor Author

Currently, coverall is executed in every job. Perhaps can migrate to a dedicated job?

@SukkaW
Copy link
Member

SukkaW commented Jul 12, 2020

Perhaps can migrate to a dedicated job?

+1

- but disable parallel mode in nyc
  * istanbuljs/nyc#1328
- pass command-line option via npm
  * https://stackoverflow.com/a/14404223
@curbengh curbengh force-pushed the parallel-mocha branch 2 times, most recently from aff32b1 to 5359528 Compare July 17, 2020 03:59
@coveralls
Copy link

coveralls commented Jul 17, 2020

Coverage Status

Coverage decreased (-0.03%) to 98.238% when pulling 110cdc6 on curbengh:parallel-mocha into 5b2808c on hexojs:master.

@curbengh curbengh marked this pull request as ready for review July 17, 2020 04:02
@curbengh curbengh marked this pull request as draft July 17, 2020 04:09
run: npm run test
env:
CI: true
coverage:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in previous draft, this job is integrated into tester job with if: ${{ matrix.os == 'ubuntu-latest' && matrix.node-version == '12.x' }}, but then tester would have one extra mocha execution which defeats the purpose.

@curbengh curbengh marked this pull request as ready for review July 17, 2020 04:21
@curbengh curbengh requested a review from SukkaW July 17, 2020 04:21
Copy link
Member

@SukkaW SukkaW left a comment

Choose a reason for hiding this comment

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

LGTM!

@SukkaW SukkaW merged commit 064daa4 into hexojs:master Jul 17, 2020
@curbengh curbengh deleted the parallel-mocha branch July 19, 2020 04:51
@SukkaW SukkaW mentioned this pull request Jul 25, 2020
22 tasks
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.

3 participants