Merge #11771: [tests] Change invalidtxrequest to use BitcoinTestFramework
ClosedPublic
Actions

Authored by markblundeberg on Jul 1 2019, 03:12.

Details

Reviewers

deadalnix
jasonbcox
Fabien

Group Reviewers

Restricted Project

Maniphest Tasks

T669: Removing comparison test framework

Commits

rSTAGINGd1c4b8b1c40c: Merge #11771: [tests] Change invalidtxrequest to use BitcoinTestFramework
rABCd1c4b8b1c40c: Merge #11771: [tests] Change invalidtxrequest to use BitcoinTestFramework

Summary

PR11771 backport https://github.com/bitcoin/bitcoin/pull/11771/files
95e2e9a [tests] Change invalidtxrequest to use BitcoinTestFramework (John Newbery)
359d067 [tests] Fix flake8 warnings in invalidtxrequest (John Newbery)
c32cf9f [tests] Add P2PDataStore class (John Newbery)
cc046f6 [tests] Reduce NodeConn connection logging from info to debug (John Newbery)

Pull request description:

Next step in #10603

- first commit changes log level for an internal log from INFO to DEBUG. (Not really related, but I started finding the INFO level logging annoying when debuging test failures)
- second commit introduces a `P2PStub` class - a subclass of `NodeConnCB` which has its own block and tx store and responds appropriately to getdata requests. Not all the functionality is used in `invalidtxrequest.py`, but will be used in `invalidblockrequest.py` and `p2p-fullblocktest` when those are changed to use `BitcoinTestFramework`
- third commit tidies up `invalidtxrequest.py`
- fourth commit removes usage of `ComparisonTestFramework`

Backport note: the "Invalid OP_IF construction" was changed to a pushonly failure.

Depends on D3501

Test Plan

test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Branch

invalidtxrequest

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6594
Build 11235: Bitcoin ABC Buildbot (legacy)
Build 11234: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jul 1 2019, 03:12

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 1 2019, 03:12

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6579: Diff 9833.Jul 1 2019, 03:19

markblundeberg added a child revision: D3495: Merge #12861: [tests] Stop feature_block.py from blowing up memory..Jul 1 2019, 03:34

Fabien requested changes to this revision.Jul 1 2019, 10:10

Fabien added inline comments.

test/functional/p2p_invalid_tx.py
5 ↗	(On Diff #9833)	Any idea why this is not part of our codebase ?
8 ↗	(On Diff #9833)	Keep the previous format, this is a regression
23 ↗	(On Diff #9833)	Move comment above
37 ↗	(On Diff #9833)	The inline comment from the PR seems true, this variable is never used.
test/functional/test_framework/mininode.py
29 ↗	(On Diff #9833)	Nit: ordering

This revision now requires changes to proceed.Jul 1 2019, 10:10

markblundeberg added a task: T669: Removing comparison test framework.Jul 1 2019, 15:03

markblundeberg added inline comments.Jul 1 2019, 15:09

test/functional/p2p_invalid_tx.py
5 ↗	(On Diff #9833)	#9577 was not backported. I'll take care of that now ...

markblundeberg added inline comments.Jul 1 2019, 16:01

test/functional/p2p_invalid_tx.py
5 ↗	(On Diff #9833)	Man that was miserable: D3501

markblundeberg marked 4 inline comments as done.Jul 1 2019, 16:44

markblundeberg added inline comments.

test/functional/p2p_invalid_tx.py
37 ↗	(On Diff #9833)	Yeah, I'll leave that for the follow up backport -- https://github.com/bitcoin/bitcoin/pull/11772/files
test/functional/test_framework/mininode.py
29 ↗	(On Diff #9833)	Yeah I noticed the linter is not watching for this stuff. I'll put them in the same order that core currently has...