Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGINGa05bfba6a467: [qa] parallelize tests suite rather than polling subprocesses
rABCa05bfba6a467: [qa] parallelize tests suite rather than polling subprocesses

Summary

Currently, the test suite runs several processes, and polls them repeatedly for completion while printing dots. This limits what kind of status updates can be provided to developers running the test suite.

This commit refactors the test runner to have several python threads which pass messages up to a status thread. This allows keeping track of which tests are running and reporting on that.

Additionally, this opens up the ability to directly import test files in the future while running them in parallel. That will enable printing detailed messages about which subtests are running rather than being per file.

Output now looks like the following:

bip68-112-113-p2p.py started
rpcbind_test.py skipped
abandonconflict.py started
Running jobs: bip68-112-113-p2p.py, p2p-compactblocks.py, abandonconflict.py, sendheaders.py
abandonconflict.py passed, Duration: 12 s
mempool_persist.py started

Note "Running jobs" rather than dots.

Test Plan

./test/functional/test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Branch

fix-stupid-rpc-tests-job-shit

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2558
Build 3233: Bitcoin ABC Buildbot (legacy)
Build 3232: arc lint + arc unit

Event Timeline

schancel created this revision.May 9 2018, 04:01

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 9 2018, 04:01

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Harbormaster completed remote builds in B2557: Diff 3833.May 9 2018, 04:26

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Clean up logging behavior

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2558: Diff 3836!May 9 2018, 07:43

Fix queue get timeout

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2559: Diff 3837.May 9 2018, 09:08

What does it fix ?

schancel retitled this revision from [qa] fix up test_runner to print better status updates to [qa] parallelize tests suite rather than polling subprocesses.May 9 2018, 20:25

schancel edited the summary of this revision. (Show Details)

schancel edited the summary of this revision. (Show Details)May 9 2018, 20:34

Ensure port seed is maintained the entire run

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Harbormaster completed remote builds in B2570: Diff 3860.May 9 2018, 23:25

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

It's very difficult to review this properly because it is shuffling a ton of code around while also introducing important logic changes.

I suggest you extract the existing logic in execute_test_processes in one diff and then modify it to use the new logic. It'll become very apparent what the new logic it and would make review significantly easier.

test/functional/test_runner.py
404 ↗	(On Diff #3860)	So this moved from the red block in the upper left.
419 ↗	(On Diff #3860)	Why does this needs to be moved ? Was something changed ? It is not possible to see right now.

This revision now requires changes to proceed.May 24 2018, 13:55

schancel added inline comments.May 24 2018, 14:56

test/functional/test_runner.py
432 ↗	(On Diff #3860)	This makes it not output anything on CI. This would be where the timeout is though.

schancel added inline comments.May 25 2018, 00:26

test/functional/test_runner.py
404 ↗	(On Diff #3860)	Yes, it's pretty necessary to do this all at once.
419 ↗	(On Diff #3860)	It doesn't need to be moved, but I prefer classes to be before their usage within a file.