[backport#16898] test: Remove connect_nodes_bi
AbandonedPublic
Actions

Authored by majcosta on Jun 4 2020, 21:20.

Details

Reviewers

deadalnix
jasonbcox

Group Reviewers

Restricted Project

Summary

test: Reformat python imports to aid scripted diff
https://github.com/bitcoin/bitcoin/pull/16898/commits/1111bb91f517838e5b9f778bf6b5a9c8d561e857

test: Use connect_nodes when connecting nodes in the test_framework
https://github.com/bitcoin/bitcoin/pull/16898/commits/faaee1e39a91b3f603881655d3980c29af09852b

scripted-diff: test: Replace connect_nodes_bi with connect_nodes

-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e
's/connect_nodes_bi\(self.nodes,\s*(.),\s*/connect_nodes(self.nodes[\1],
/g' $(git grep -l connect_nodes_bi)
sed -i --regexp-extended -e 's/connect_nodes_bi(,| )/connect_nodes\1/g'
$(git grep -l connect_nodes_bi)
-END VERIFY SCRIPT-

https://github.com/bitcoin/bitcoin/pull/16898/commits/fa3b9ee8b2280af4bcbcfffff275aaf8dd125929

test: Remove unused connect_nodes_bi
https://github.com/bitcoin/bitcoin/pull/16898/commits/fadfd844de8c53034a97dfa6f771ffe9f523fba2

Had to adapt changes to our connect_nodes API that does not take an int for second parameter

abc-parkedchain was failing, seemed like one of the nodes was banning the other partway thru the test. abc-finalize-block also failed, seemed it depended on the old behavior of test_framework.py's setup_network(), changed it so that alt_node connects to node and not the other way around

Backport of Core PR16898

Test Plan

ninja
./test/functional/test_runner.py --extended

Diff Detail

Repository

rABC Bitcoin ABC

Branch

p16898

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 11214
Build 21107: Default Diff Build & Tests
Build 21106: Build without Wallet
Build 21105: Build with clang-10
Build 21104: arc lint + arc unit

Event Timeline

majcosta created this revision.Jun 4 2020, 21:20

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 4 2020, 21:20

majcosta requested review of this revision.Jun 4 2020, 21:20

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Snippet of first build failure:

2020-06-04T21:26:19.177000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] wallet_resendwallettransactions.py
[21:27:05]i:			 [wallet_resendwallettransactions.py] Test ignored: wallet_resendwallettransactions.py
[21:27:05]i:		 [Bitcoin ABC functional tests] [Test Output]
2020-06-04T21:26:18.856000Z TestFramework (WARNING): Test Skipped: wallet has not been compiled.
2020-06-04T21:26:18.906000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:26:18.906000Z TestFramework (INFO): Cleaning up /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/wallet_resendwallettransactions_34 on exit
2020-06-04T21:26:18.907000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] wallet_txn_clone.py
[21:27:05]i:			 [wallet_txn_clone.py] Test ignored: wallet_txn_clone.py
[21:27:05]i:		 [Bitcoin ABC functional tests] [Test Output]
2020-06-04T21:26:54.148000Z TestFramework (WARNING): Test Skipped: wallet has not been compiled.
2020-06-04T21:26:54.198000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:26:54.209000Z TestFramework (INFO): Cleaning up /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/wallet_txn_clone_101 on exit
2020-06-04T21:26:54.209000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] wallet_txn_clone.py --mineblock
[21:27:05]i:			 [wallet_txn_clone.py --mineblock] Test ignored: wallet_txn_clone.py --mineblock
[21:27:05]i:		 [Bitcoin ABC functional tests] [Test Output]
2020-06-04T21:26:46.868000Z TestFramework (WARNING): Test Skipped: wallet has not been compiled.
2020-06-04T21:26:46.918000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:26:46.918000Z TestFramework (INFO): Cleaning up /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/wallet_txn_clone_82 on exit
2020-06-04T21:26:46.918000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] wallet_txn_doublespend.py
[21:27:05]i:			 [wallet_txn_doublespend.py] Test ignored: wallet_txn_doublespend.py
[21:27:05]i:		 [Bitcoin ABC functional tests] [Test Output]
2020-06-04T21:27:01.182000Z TestFramework (WARNING): Test Skipped: wallet has not been compiled.
2020-06-04T21:27:01.232000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:27:01.232000Z TestFramework (INFO): Cleaning up /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/wallet_txn_doublespend_134 on exit
2020-06-04T21:27:01.232000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] wallet_txn_doublespend.py --mineblock
[21:27:05]i:			 [wallet_txn_doublespend.py --mineblock] Test ignored: wallet_txn_doublespend.py --mineblock
[21:27:05]i:		 [Bitcoin ABC functional tests] [Test Output]
2020-06-04T21:26:47.003000Z TestFramework (WARNING): Test Skipped: wallet has not been compiled.
2020-06-04T21:26:47.054000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:26:47.054000Z TestFramework (INFO): Cleaning up /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/wallet_txn_doublespend_83 on exit
2020-06-04T21:26:47.054000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] wallet_zapwallettxes.py
[21:27:05]i:			 [wallet_zapwallettxes.py] Test ignored: wallet_zapwallettxes.py
[21:27:05]i:		 [Bitcoin ABC functional tests] [Test Output]
2020-06-04T21:26:45.308000Z TestFramework (WARNING): Test Skipped: wallet has not been compiled.
2020-06-04T21:26:45.359000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:26:45.359000Z TestFramework (INFO): Cleaning up /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/wallet_zapwallettxes_71 on exit
2020-06-04T21:26:45.359000Z TestFramework (INFO): Test skipped
[21:27:05]i:		 [Bitcoin ABC functional tests] p2p_timeouts.py
[21:27:05]i:			 [p2p_timeouts.py] [Test Output]
2020-06-04T21:26:38.180000Z TestFramework (INFO): Initializing test directory /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/p2p_timeouts_62
2020-06-04T21:26:43.495000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 183, in main
    self.run_test()
  File "/work/test/functional/p2p_timeouts.py", line 84, in run_test
    assert not no_verack_node.is_connected
AssertionError
2020-06-04T21:26:43.879000Z TestFramework (INFO): Stopping nodes
2020-06-04T21:26:44.081000Z TestFramework (WARNING): Not cleaning up dir /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/p2p_timeouts_62
2020-06-04T21:26:44.081000Z TestFramework (ERROR): Test failed. Test logging available at /work/build/test/tmp/test_runner_₿₵_  _20200604_212617/p2p_timeouts_62/test_framework.log
2020-06-04T21:26:44.082000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/build/test/tmp/test_runner_₿₵_  _20200604_212617/p2p_timeouts_62' to consolidate all logs
[21:27:07]E:	 [Step 1/2] Failed tests detected
[21:27:11]W:	 [Step 1/2] Process exited with code 1
[21:27:11]E:	 [Step 1/2] Process exited with code 1 (Step: Command Line)

Each failure log is accessible here:
Bitcoin ABC functional tests: p2p_timeouts.py

Harbormaster completed remote builds in B11214: Diff 20921.Jun 4 2020, 21:27

majcosta planned changes to this revision.Jun 4 2020, 21:40

Ok, the main problem here is that this completely unreviewable. You have a set of manual changes that are swamped into a shotgun automated refactoring. You need to decouple the manual changes and the automated ones. It'll also make it more evident why some of the changes to abc tests are necessary, because they seems to be at best fishy from where I'm at.