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.

Sign up for GitHub

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

Speed up tests locally by upto 45% #1818

Merged
merged 2 commits into from
Sep 24, 2023
Merged

Speed up tests locally by upto 45% #1818

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee7bfae
Update package.json
anshgoyalevil Sep 24, 2023
f658eb1
Merge branch 'main' into boost-tests-locally
anshgoyalevil Sep 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Update package.json 
Signed-off-by: Ansh Goyal <[email protected]>
  • Loading branch information
@anshgoyalevil
anshgoyalevil authored Sep 24, 2023
commit ee7bfaebe74af045abd2f2b11935818a8a6b8c9f
2 changes: 1 addition & 1 deletion packages/jaeger-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
"coverage": "yarn run test-ci --coverage",
"start:ga-debug": "REACT_APP_GA_DEBUG=1 REACT_APP_VSN_STATE=$(../../scripts/get-tracking-version.js) vite",
"start": "vite",
"test": "jest",
"test": "jest --maxWorkers=50%",
Copy link
Member

Choose a reason for hiding this comment

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

does % refer to the number of available CPU cores?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It's the CPU Cores present in the current run environment. Most of the articles on internet recommends this setting.

Copy link
Member

@yurishkuro yurishkuro Sep 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense for local testing, so as not to bring the whole system down during the test. But if this %-based addressing scheme is available, why does the line below have --maxWorkers=4? We don't really know how many CPU cores are available in a CI runner, but it would make sense to utilize all of them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Every CI environment has their own workers limit, and the setting which we are currently using for test-ci is adapted to GitHub Actions only.

Moreover, reason for choosing a concrete value (4) instead of % over here is because in CI, we know we won't be performing multi-tasking, so all threads can be used for tests. (Minus the thread for main workflow task)

In future, if we have any plans to move to another CI environment like CircleCI, I think we would again have to figure out this max number of workers. (I guess, it's 8 for CircleCI).

Copy link
Member

Choose a reason for hiding this comment

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

But why can't we say 100% for GitHub? When the tests are running, no other activity is higher priority.

Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically that seems right. Don't know why everyone suggests using a number value instead of percentage in CI for jest.

I would have to try that thing.

"test-ci": "CI=1 jest --silent --color --maxWorkers=4"
},
"jest": {
Expand Down