qa: Remove polling loop from test_runner#13384
qa: Remove polling loop from test_runner#13384maflcko wants to merge 1 commit intobitcoin:masterfrom
Conversation
maflcko
force-pushed
the
Mf1806-qaTestRunnerConcurrentFuture
branch
2 times, most recently
from
facf1a0 to
fa30a1d
Compare
maflcko
force-pushed
the
Mf1806-qaTestRunnerConcurrentFuture
branch
from
fa30a1d to
fa35a02
Compare
|
I don't see difference with regard to run time: However I like the code cleanup and it doesn't make it worse so |
|
Seems that with this change, Ctrl-C does no longer quit the test runner immediately when the tests have started, looks like it has to be pressed twice.
I suddenly remember that this was added to prevent Travis from timing out due to no output. Is that no longer a concern? |
The run time is between 2 and 3 minutes. And travis would only time out after 10 minutes of no dots received. The extended tests are not run on travis for a couple of weeks now, so I can't answer that... |
|
Closing for now. Not really worth the effort |
|
Concept ACK. I'd considered replacing our custom-made process pool with Even if you don't include that change, it seems like some of the tidy-up in this PR is useful (eg the |
3 participants
My initial design for parallel tests was a busy loop (busy waiting) that polls the running processes every half a second.
Using pythons own
ThreadPoolExecutormakes the code smaller and hopefully easier to read/understand. By removing the half-a-second overhead, I believe the tests run now slightly faster (Verified on my workstation, but should also be verified by one of the reviewers, ideally)This is refactoring beside the following changes:
--failfastwill now wait for unfinished jobs (similar to a failure inmake)portseed_offsetis no longer applied. This is no longer required since we already kill leftover processes (see [qa] Ensure bitcoind processes are cleaned up when tests end #12904). This "fixes" Travis: failed to bind to port 11211 #10869 because we deterministically pick ports starting at 11000