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

Enable higher parallelism in tests #18744

Open
wknapik opened this issue Oct 13, 2021 · 6 comments
Open

Enable higher parallelism in tests #18744

wknapik opened this issue Oct 13, 2021 · 6 comments
Labels
ci-concern dev-concern OS/Android Fixes related to Android browser functionality OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains.
Projects

Comments

@wknapik
Copy link
Contributor

wknapik commented Oct 13, 2021

Tests run with --test-launcher-jobs=4 by default, including in CI. In Windows/ASAN we had to go down to 2 for unit tests, otherwise they wouldn't pass.

Relevant slack threads:

Failures with higher parallelism indicate real problems, like missing dependencies, race conditions, etc. And with a growing number of tests, this will keep lengthening the feedback loop (both in CI and locally), which we need to shorten.

@wknapik wknapik added dev-concern OS/Android Fixes related to Android browser functionality OS/Desktop ci-concern labels Oct 13, 2021
@wknapik
Copy link
Contributor Author

wknapik commented Nov 3, 2021

Also, retries built into the testware hide ASAN findings (https://bravesoftware.slack.com/archives/C6R461GF4/p1636537519052500). Taking out those retries would likely cause every run to fail. That applies at least to browser tests.

@mihaiplesa mihaiplesa added this to To do in CI concerns Dec 28, 2021
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Apr 11, 2023
@rebron
Copy link
Collaborator

rebron commented Apr 11, 2023

cc: @bsclifton Need sizing for this one and when we can possibly schedule it.

@wknapik
Copy link
Contributor Author

wknapik commented May 25, 2023

Things have improved since this was reported

Parallelism is now increased across platforms by explicitly passing --test-launcher-jobs every time npm run test is called

Linux

  • asan:
    • but: 4 -> nproc (48)
    • cut: 28 -> nproc (48)
    • bbt: 4 -> nproc (48)
    • cbt: 20 -> nproc (48)
  • prs:
    • but: 4 -> nproc (48)
    • cut: 28 -> nproc (48)
    • bbt: 4 -> nproc (48)
    • cbt: 20 -> nproc (48)
  • test and public builds:
    • but: 4 -> nproc (48)
    • bbt: 4 -> nproc (48)

macOS

  • asan:
    • but: 4 -> nproc
    • bbt: 4 -> nproc
  • prs:
    • but: 4 -> nproc
    • bbt: 4 -> nproc
  • test and public builds:
    • but: 4 -> nproc
    • bbt: 4 -> nproc

Windows

  • asan:
    • but: 2 -> nproc (32)
    • cut: 16 -> nproc (32)
    • bbt: 4 -> nproc/2 (16)
    • cbt: 8 -> nproc/3 (10)
  • prs:
    • but: 4 -> nproc (32)
    • cut: 16 -> nproc (32)
    • bbt: 4 -> nproc/2 (16)
    • cbt: 16 -> nproc/2 (16)
  • test and public builds:
    • but: 4 -> nproc (32)
    • bbt: 4 -> nproc/2 (16)

Android

  • prs:
    • but: 4 -> nproc (96)
    • bbt: 4 -> nproc (96)
  • test and public builds
    • but: 4 -> nproc (96)
    • bbt: 4 -> nproc (96)

iOS

  • prs:
    • ? -> nproc
  • test and public builds
    • ? -> nproc

@wknapik
Copy link
Contributor Author

wknapik commented May 25, 2023

In the course of testing the above changes, it became clear that, at least on desktop, there's an internal limit to --test-launcher-jobs. Running with the value of 1024 doesn't produce any failures as it would be expected to (except in browser tests on Windows). The log says either "Using 1024 parallel jobs", or "Using 512 parallel jobs".

@wknapik
Copy link
Contributor Author

wknapik commented May 25, 2023

Also, --test-launcher-bot-mode does not set --test-launcher-jobs even when the latter is not passed explicitly to npm run test. Might be because we set a default of 4.

@wknapik
Copy link
Contributor Author

wknapik commented Jun 27, 2023

We now use --test-launcher-jobs=nproc across platforms and test types, except Windows ASAN builds, where it's nproc/1.5 for brave browser tests and nproc/3 for chromium browser tests (otherwise memory runs out)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-concern dev-concern OS/Android Fixes related to Android browser functionality OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains.
Projects
CI concerns
  
Backlog
Development

No branches or pull requests

2 participants