Merge #12926: Run unit tests in parallel
ClosedPublic
Actions

Authored by markblundeberg on Jul 3 2019, 00:28.

Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGING2fa11c7cc7ec: Merge #12926: Run unit tests in parallel
rABC2fa11c7cc7ec: Merge #12926: Run unit tests in parallel

Summary

PR12926 backport https://github.com/bitcoin/bitcoin/pull/12926/files
7ef9cd8 Increase entropy in test temp directory name (Pieter Wuille)
f6dfb0f Reorder travis builds (Pieter Wuille)
156db42 tests: run tests in parallel (Cory Fields)
66f3255 tests: split up actual tests and helper files (Cory Fields)

Pull request description:

This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32).

This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure.

Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests.

Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously.

This is an alternative to #12831.

Test Plan

make check and pay attention to the output

Diff Detail

Repository

rABC Bitcoin ABC

Branch

patch

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6647
Build 11341: Bitcoin ABC Buildbot (legacy)
Build 11340: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jul 3 2019, 00:28

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 3 2019, 00:28

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6647: Diff 9945.Jul 3 2019, 00:34

This is pretty neat but it still has some issues.

The testsuite summary prints out too early! (with TOTAL: 0).
It no longer runs in parallel with the secp256k1 tests!

As a result it's barely faster:

$ time make -j4 check
...
real	2m15.085s
user	4m5.065s
sys	1m19.582s

Before:

$ time make -j4 check
...
real	2m31.073s
user	4m0.694s
sys	1m15.051s

markblundeberg edited the summary of this revision. (Show Details)Jul 3 2019, 00:46

If we take this, we should also do this fix for OpenBSD compat (why not): https://github.com/bitcoin/bitcoin/pull/13355

Note that if accepted, this (and the trivial fix mentioned) would bring us up to sync with Core regarding the way tests are done. All other changes in this makefile seem to be regarding fuzzing.

So, maybe we could do it a better way...

We could benefit from properly using the parallel test harness:
https://www.gnu.org/software/automake/manual/html_node/Parallel-Test-Harness.html

If TESTS= were filled up with all the individual tests, then they would run in parallel, and also run parallel to the secp256k1 tests. Moreover, they would get properly logged together into the test-suite.log AND the test summary would be informative. I tried playing around with automake to achieve this but I gave up quickly, as I don't really understand automake.

Fabien notes that the test summary actually prints out one test: the bitcoin-qt tests. For me it was 0 because I was using a non-qt build.

Agreed that there are better solutions, but this is an improvement with few drawbacks.
I'll take care to get cmake up with something similar.

This revision is now accepted and ready to land.Jul 3 2019, 17:49

Closed by commit rABC2fa11c7cc7ec: Merge #12926: Run unit tests in parallel (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com>). · Explain WhyJul 3 2019, 18:18

This revision was automatically updated to reflect the committed changes.

Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rABC2fa11c7cc7ec: Merge #12926: Run unit tests in parallel.

markblundeberg mentioned this in D3540: make check in parallel - turbo mode.Jul 3 2019, 19:03

Fabien mentioned this in D3591: Move make check test logs out of tree.Jul 8 2019, 09:39

Fabien mentioned this in rABC7630f24afa02: Move make check test logs out of tree.Jul 8 2019, 15:58

Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rSTAGING2fa11c7cc7ec: Merge #12926: Run unit tests in parallel.Sep 19 2019, 00:55

Fabien mentioned this in rSTAGING7630f24afa02: Move make check test logs out of tree.Sep 19 2019, 01:03

Fabien mentioned this in D4698: [CMAKE] Allow to run boost unit tests in parallel.Dec 11 2019, 16:52