Skip to content

ci: Run unit tests parallel with functional tests #33000

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

Jump to bottom
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

ci: Run unit tests parallel with functional tests #33000

wants to merge 1 commit into from
+51 −8

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jul 17, 2025

Fixes #32770

@maflcko maflcko force-pushed the 2507-ci-fun branch 2 times, most recently from a1c66c6 to fa8aa95 Compare July 17, 2025 11:33
@maflcko maflcko mentioned this pull request Jul 17, 2025
Open
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 17, 2025 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33000.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK jonatack
Concept ACK l0rinc, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #33136 (ci: Remove redundant RUN_UNIT_TESTS_SEQUENTIAL by maflcko)
  • #32989 (ci: Migrate CI to hosted Cirrus Runners by willcl-ark)
  • #31679 (cmake: Install internal binaries to /libexec/ by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko maflcko added the Tests label Jul 17, 2025
@maflcko
Copy link
Member Author

maflcko commented Jul 17, 2025

Looks like this gives a 20% faster Msan, Tsan, and previous releases task with N=1, comparing with the fastest master run in the last week:

This was referenced Jul 17, 2025
Open
Merged
jonatack
jonatack reviewed Jul 25, 2025
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

I don't know how much variance we typically see in CI run times, but light lgtm ACK fab25c6

$ /test_runner.py -h
  --valgrind            run nodes under the valgrind memory error detector: expect at least a ~10x slowdown. valgrind 3.14 or later required. Does not apply to previous release
                        binaries.
  --bash-cmd-extra-tests BASH_CMD_EXTRA_TESTS
                        Run an arbitrary string as Bash command in parallel to the functional tests, for example to run unit tests. See tool_extra_cmd.py
  --randomseed RANDOMSEED
                        set a random seed for deterministically reproducing a previous test run

$ ./build/test/functional/tool_extra_cmd.py
2025-07-25T17:15:46.612122Z TestFramework (INFO): PRNG seed is: 5551363100202563085
2025-07-25T17:15:46.612554Z TestFramework (INFO): Initializing test directory /var/folders/bz/mn3hr6td37bczwp7j89frs4w0000gn/T/bitcoin_func_test_c89awace
2025-07-25T17:15:46.612626Z TestFramework (INFO): No command to run.
2025-07-25T17:15:46.667850Z TestFramework (INFO): Stopping nodes
2025-07-25T17:15:46.667957Z TestFramework (INFO): Cleaning up /var/folders/bz/mn3hr6td37bczwp7j89frs4w0000gn/T/bitcoin_func_test_c89awace on exit
2025-07-25T17:15:46.667990Z TestFramework (INFO): Tests successful

@l0rinc
Copy link
Contributor

l0rinc commented Jul 26, 2025

Concept ACK

@fanquake
Copy link
Member

Concept ACK. Not sure about the approach of more Bash, to call new Python, that wraps more Bash.

@maflcko maflcko marked this pull request as draft July 28, 2025 07:32
@maflcko
Copy link
Member Author

maflcko commented Jul 28, 2025

The implementation can be adjusted, if needed. I guess the main questions are:

  • Do people want to use this locally, if yes, then there could be a more user friendly way of calling it?
  • Does it help with the new CI? With the old CI system planned to be replaced, it makes little sense to fine-tune it this late, so I'll turn this into a draft for now.

@fanquake
Copy link
Member

Does it help with the new CI? With the old CI system planned to be replaced, it makes little sense to fine-tune it this late, so I'll turn this into a draft for now.

cc @m3dwards @willcl-ark

@maflcko
Copy link
Member Author

maflcko commented Jul 28, 2025

I'd say it should be done after the move. No need to delay the move or block it on this.

@maflcko maflcko changed the title ci: Run unit test parallel with functional tests ci: Run unit tests parallel with functional tests Aug 5, 2025
@DrahtBot DrahtBot mentioned this pull request Aug 5, 2025
Open
@maflcko
Copy link
Member Author

maflcko commented Aug 6, 2025

Concept ACK. Not sure about the approach of more Bash, to call new Python, that wraps more Bash.

Yeah, it is ugly. I think I'll drop some of the (actually redundant) Bash in #33136 first. Then, this can be done in pure Python, possibly.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 7, 2025

🐙 This pull request conflicts with the target branch and needs rebase.

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

@jonatack jonatack jonatack left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Needs rebase Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Slow unit tests delay functional tests and leave CPU unused
5 participants
@maflcko @DrahtBot @l0rinc @fanquake @jonatack