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

feat: make tests run in parallel #1312

Merged
merged 11 commits into from
Nov 9, 2023

Conversation

zivkovicmilos
Copy link
Member

@zivkovicmilos zivkovicmilos commented Oct 27, 2023

Description

This PR simply introduces t.Parallel() for unit tests that can support it.

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@zivkovicmilos zivkovicmilos self-assigned this Oct 27, 2023
@zivkovicmilos zivkovicmilos requested review from jaekwon, moul and a team as code owners October 27, 2023 15:32
@github-actions github-actions bot added 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Oct 27, 2023
@zivkovicmilos zivkovicmilos marked this pull request as draft October 27, 2023 15:39
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c33c9f2) 55.96% compared to head (e747fd1) 55.97%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1312   +/-   ##
=======================================
  Coverage   55.96%   55.97%           
=======================================
  Files         420      420           
  Lines       65380    65380           
=======================================
+ Hits        36592    36598    +6     
+ Misses      25932    25926    -6     
  Partials     2856     2856           

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zivkovicmilos zivkovicmilos marked this pull request as ready for review October 27, 2023 17:42
Copy link
Member

@gfanton gfanton left a comment

Choose a reason for hiding this comment

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

nice ! Do you have an after and before comparaison for test time ?

@zivkovicmilos
Copy link
Member Author

zivkovicmilos commented Nov 9, 2023

nice ! Do you have an after and before comparaison for test time ?

@gfanton
I ran it locally on my tin-can i5 mac:

master branch:

make test.components 2>&1  1686.41s user 88.84s system 111% cpu 26:26.36 total
tee test.log  0.07s user 1.67s system 0% cpu 26:26.36 total

feat/parallel-tests branch:

make test.components 2>&1  1829.86s user 129.74s system 144% cpu 22:35.59 total
tee test.log  0.08s user 1.80s system 0% cpu 22:35.59 total

So ~4mins faster 🚀

(ignore the tee, I use it to capture any random test functionality)

Keep in mind our CI workflows are ran in parallel, and this is just a local test run; We should however see a slight increase in CI run speed

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

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

Only touching tests and already approved multiple times, so merging!

I made an rg command to look for tests which still are not parallel (will contain false positives, but good if anyone wants to pick up your efforts:

rg --iglob '*_test.go' --pcre2 --sort=path -r '$1' -o -U 'func (Test[^\(]*)\(.*\n\t(?!t\.Parallel)'

🎉

@thehowl thehowl merged commit e803c45 into gnolang:master Nov 9, 2023
194 checks passed
@zivkovicmilos zivkovicmilos deleted the feat/parallel-tests branch November 9, 2023 14:10
moul pushed a commit to moul/gno that referenced this pull request Nov 14, 2023
## Description

This PR simply introduces `t.Parallel()` for unit tests that can support
it.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Guilhem Fanton <[email protected]>
@moul moul mentioned this pull request Jul 3, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants