Page MenuHomePhabricator

Merge #13517: qa: Remove need to handle the network thread in tests
ClosedPublic

Authored by jasonbcox on Thu, Sep 5, 23:57.

Details

Summary

Merge #13517: qa: Remove need to handle the network thread in tests
fa87da2f172ae2e6dc15e9ed156a3564a8ecfbdd qa: Avoid start/stop of the network thread mid-test (MarcoFalke)

Pull request description:

This simplifies test writing by removing the need to handle the network thread in tests. E.g. start thread, join thread, restart thread mid-test, adding p2p connections at the "right" time, ...

Tree-SHA512: 533642f12fef5496f1933855edcdab1a7ed901d088d34911749cd0f9e044c8a6cb1f89985ac3a7f41a512943663e4e270a61978f6f072143ae050cd102d4eab8

Backport of Core PR13517
https://github.com/bitcoin/bitcoin/pull/13517/files
Completes T676
Depends on D4002, D4011


Merge #13715: tests: fixes mininode's P2PConnection sending messages on closing transport
ea5340c9d2 tests: fixes mininode's P2PConnection sending messages on closing transport (marcoagner)

Pull request description:

Fixes #13579.
I think one possible solution is to check for [`_transport.is_closing()`](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing) in the lambda before sending a message (compatible with Python 3.4 too). Let me know if I missed any side effects this introduces.

Tree-SHA512: cab46f81dccfec7b4460fda478a617845564520694449a9e85bf8a5f1e75f35f52cafd7c64966712c3d6c29956344d5a9dbad8851424f061eb3748bc621b900b

Backport of Core PR13715
https://github.com/bitcoin/bitcoin/pull/13715/files
Completes T678


Both backports included because feature_assumevalid.py breaks without PR13715

Test Plan

test_runner.py

Diff Detail

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

Event Timeline

jasonbcox created this revision.Thu, Sep 5, 23:57
Fabien requested changes to this revision.Fri, Sep 6, 07:21
Fabien added a subscriber: Fabien.

This is causing feature_assumevalid to fail with socket.send() raised exception.. This is a repeatable issue.

This revision now requires changes to proceed.Fri, Sep 6, 07:21
Fabien added a comment.Fri, Sep 6, 07:23

I've just seen D4013 which fixes the issue. Since it's a one liner please squash to avoid introducing test failure.

jasonbcox updated this revision to Diff 11123.Fri, Sep 6, 15:14

Merged in D4013 to fix flaky feature_assumevalid

jasonbcox edited the summary of this revision. (Show Details)Fri, Sep 6, 15:46
jasonbcox edited the summary of this revision. (Show Details)
Fabien accepted this revision.Fri, Sep 6, 15:51
This revision is now accepted and ready to land.Fri, Sep 6, 15:51