Page MenuHomePhabricator

Merge #12926: Run unit tests in parallel

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



PR12926 backport
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

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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


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

If we take this, we should also do this fix for OpenBSD compat (why not):

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:

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