Page MenuHomePhabricator
Differential D3514

Merge #11773: [tests] Change feature_block.py to use BitcoinTestFramework
ClosedPublic
Actions

Authored by markblundeberg on Jul 2 2019, 06:34.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Project
Maniphest Tasks
T669: Removing comparison test framework
Commits
rSTAGING2e1d21e29394: Merge #11773: [tests] Change feature_block.py to use BitcoinTestFramework
rABC2e1d21e29394: Merge #11773: [tests] Change feature_block.py to use BitcoinTestFramework
Summary

PR11773 backport https://github.com/bitcoin/bitcoin/pull/11773/files
265d7c4 [tests] Improve assert message when wait_until() fails (John Newbery)
ebf053a [tests] Change feature_block.py to use BitcoinTestFramework (John Newbery)
fc02c12 [tests] Add logging to feature_block.py (John Newbery)
3898c4f [tests] Tidy up feature_block.py (John Newbery)
5cd01d2 [tests] Fix flake8 warnings in feature_block.py (John Newbery)

Pull request description:

Next step in #10603.

- first three commits tidy up feature_block.py
- fourth commit removes usage of ComparisonTestFramework

Longer term, it would be better to separate net_processing testing from validation testing, but I think this is still a useful PR, since it moves us away from the comparison test framework.
Test Plan

test_runner.py feature_block.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR11773
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6638
Build 11323: Bitcoin ABC Buildbot (legacy)
Build 11322: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jul 2 2019, 06:34
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 2 2019, 06:34
teamcity added a comment.Jul 2 2019, 06:34

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

markblundeberg planned changes to this revision.Jul 2 2019, 06:37

linter introduced syntax errors

markblundeberg added a task: T669: Removing comparison test framework.Jul 2 2019, 06:37
teamcity added a comment.Jul 2 2019, 06:40

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

Harbormaster completed remote builds in B6616: Diff 9901.Jul 2 2019, 06:40
markblundeberg updated this revision to Diff 9902.Jul 2 2019, 06:45

fix lint-induced syntax errors and remove lint-identified unused imports

teamcity added a comment.Jul 2 2019, 06:45

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

markblundeberg added a comment.Jul 2 2019, 06:50

This was, hands down, the most enjoyable backport ever.

(Now excuse me while I go cry myself to sleep...)

Harbormaster completed remote builds in B6617: Diff 9902.Jul 2 2019, 06:51
teamcity added a comment.Jul 2 2019, 06:51

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

markblundeberg planned changes to this revision.Jul 2 2019, 19:29

Self review the next morning shows only minor issues. Pointing out some of the changes that happened due to CTOR and the padding rule below, and some minor things to change...

test/functional/feature_block.py
172 ↗(On Diff #9902)

Not clear to me why this was needed... (from D1907)
It has been taken out of the new next_block

175 ↗(On Diff #9902)

move these back

307 ↗(On Diff #9902)

(Note: we do not have the b24/b25 too-large block rejection here.)

545 ↗(On Diff #9902)

this is different from CTOR

655 ↗(On Diff #9902)

note our error message here differs...

658 ↗(On Diff #9902)

Note to self: remove this comment! Our test here is different in function.

The duplicate check we perform here is for CTOR blocks only. Core's duplicate check is just checking merkle malleation.

750 ↗(On Diff #9902)

Our tail repeat here is different from CTOR as well...

833 ↗(On Diff #9902)

linter did this ... I guess it's nice to have

890 ↗(On Diff #9902)

Note with our large block limit, this test unfortunately does not do exactly what we want. To be fully mimicking Core here, we should make a 32 MB block that goes over with non-canonical varint.

923 ↗(On Diff #9902)

(b66 test skipped here, since we don't have TTOR it doesn't apply)

931 ↗(On Diff #9902)

typo 'creted'

1009 ↗(On Diff #9902)

different from CTOR

1018 ↗(On Diff #9902)

(this is unnecessary, right?)

1063 ↗(On Diff #9902)

This is a fascinating and scary consensus rule... makes me hate sigops even more :-D

1203 ↗(On Diff #9902)

this vout_offset business comes from pad_tx adding an extra output during create_tx

markblundeberg added a comment.Jul 2 2019, 19:42

Note to self: also backport #13048 after this

markblundeberg added a child revision: D3527: Merge #13003: qa: Add test for orphan handling.Jul 2 2019, 19:50
markblundeberg added inline comments.Jul 2 2019, 19:59
test/functional/feature_block.py
179 ↗(On Diff #9902)

This is also unnecessary. A block with only one non-coinbase transaction is always canonically ordered.

markblundeberg updated this revision to Diff 9928.Jul 2 2019, 20:14

minor changes

teamcity added a comment.Jul 2 2019, 20:14

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

teamcity added a comment.Jul 2 2019, 20:16

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

Harbormaster failed remote builds in B6635: Diff 9928!Jul 2 2019, 20:16
markblundeberg updated this revision to Diff 9930.Jul 2 2019, 20:27

just rebase

teamcity added a comment.Jul 2 2019, 20:27

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

teamcity added a comment.Jul 2 2019, 20:30

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

Harbormaster failed remote builds in B6636: Diff 9930!Jul 2 2019, 20:30
markblundeberg added a child revision: D3528: Merge #13048: [tests] Fix feature_block flakiness.Jul 2 2019, 20:34
markblundeberg updated this revision to Diff 9935.Jul 2 2019, 20:54

rebase again :/

teamcity added a comment.Jul 2 2019, 20:54

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

teamcity added a comment.Jul 2 2019, 21:00

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

Harbormaster completed remote builds in B6638: Diff 9935.Jul 2 2019, 21:00
deadalnix accepted this revision.Jul 3 2019, 13:40
This revision is now accepted and ready to land.Jul 3 2019, 13:40
Closed by commit rABC2e1d21e29394: Merge #11773: [tests] Change feature_block.py to use BitcoinTestFramework (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com>). · Explain WhyJul 3 2019, 14:47
This revision was automatically updated to reflect the committed changes.
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rABC2e1d21e29394: Merge #11773: [tests] Change feature_block.py to use BitcoinTestFramework.
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rSTAGING2e1d21e29394: Merge #11773: [tests] Change feature_block.py to use BitcoinTestFramework.Sep 19 2019, 00:53

Revision Contents
Changeset List

PathSize
test/
functional/
1275 lines
test_framework/
15 lines

Diff 9935

View Options

test/functional/feature_block.py

Loading...
View Options

test/functional/test_framework/util.py

Loading...