Skip to content

Conversation

@bajtos
Copy link
Member

@bajtos bajtos commented Jun 26, 2020

  • move Mocha flags from CLI args to config file
  • enable parallel execution of Mocha tests by adding parallel: true to monorepo's root mocha config
  • update DEVELOPING.md to mention parallel execution and to show how to run tests in series

This pull request supersedes #5011. I am intentionally keeping this change small and enabling parallel test execution only when the tests are executed for the entire monorepo. We can discuss further improvements after this pull request is landed. (E.g. which packages should run tests in parallel when running npm t from their package directory. At the moment, users can opt into parallel execution via npm t -- -p).

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@bajtos bajtos added developer-experience Issues affecting ease of use and overall experience of LB users Internal Tooling Issues related to our tooling and monorepo infrastructore labels Jun 26, 2020
@bajtos bajtos requested a review from raymondfeng June 26, 2020 08:40
@bajtos
Copy link
Member Author

bajtos commented Jun 26, 2020

@raymondfeng Few booter tests are failing in parallel mode, can you PTAL? You can reproduce the problem on master by running the following command (after npm run build):

$ npm run mocha -- --parallel

It would be great to fix the problem in a new pull request and then rebase this pull request on top of the new master.

@bajtos bajtos mentioned this pull request Jun 26, 2020
14 tasks
@bajtos
Copy link
Member Author

bajtos commented Jun 26, 2020

Perhaps #5747 is going to fix the failing boot tests?

@bajtos bajtos closed this Jun 26, 2020
@bajtos bajtos reopened this Jun 26, 2020
@raymondfeng
Copy link
Contributor

@bajtos I have rebased it to master to pick up #5747

.mocharc.js Outdated
const defaultConfig = require('./packages/build/config/.mocharc.json');

const MONOREPO_CONFIG = {
lang: 'en_US.UTF-8',
Copy link
Contributor

Choose a reason for hiding this comment

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

lb-mocha won't be able to pick lang from mocha config.

package.json Outdated
"docs:prepare": "./docs/bin/build-preview-site.sh",
"docs:start": "cd docs/_preview && bundle exec jekyll serve --no-w --i",
"mocha": "node packages/build/bin/run-mocha --lang en_US.UTF-8 --timeout 5000 \"packages/*/dist/__tests__/**/*.js\" \"extensions/*/dist/__tests__/**/*.js\" \"examples/*/dist/__tests__/**/*.js\" \"packages/cli/test/**/*.js\" \"packages/build/test/*/*.js\"",
"mocha": "node packages/build/bin/run-mocha \"packages/*/dist/__tests__/**/*.js\" \"extensions/*/dist/__tests__/**/*.js\" \"examples/*/dist/__tests__/**/*.js\" \"packages/cli/test/**/*.js\" \"packages/build/test/*/*.js\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to use --lang with the command.

bajtos added 2 commits June 29, 2020 09:24
- Remove `timeout`, it should be aggregated from individual packages.
  At the moment, `packages/cli` is already setting timeout 5000ms.

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
@bajtos bajtos requested review from agnes512 and emonddr as code owners June 29, 2020 22:08
@raymondfeng raymondfeng merged commit 7b74bd9 into master Jun 30, 2020
@raymondfeng raymondfeng deleted the test/parallel branch June 30, 2020 15:30
it('saves command metadata to .yo-rc.json', () => {
it('saves command metadata to .yo-rc.json', function () {
// This test can be slow under parallel mode
// eslint-disable-next-line @typescript-eslint/no-invalid-this
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not ideal. I opened a PR to avoid eslint-disable-next-line, see #5925

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer-experience Issues affecting ease of use and overall experience of LB users Internal Tooling Issues related to our tooling and monorepo infrastructore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants