Page MenuHomePhabricator

Merge #11773: [tests] Change to use BitcoinTestFramework

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



PR11773 backport
265d7c4 [tests] Improve assert message when wait_until() fails (John Newbery)
ebf053a [tests] Change to use BitcoinTestFramework (John Newbery)
fc02c12 [tests] Add logging to (John Newbery)
3898c4f [tests] Tidy up (John Newbery)
5cd01d2 [tests] Fix flake8 warnings in (John Newbery)

Pull request description:

Next step in #10603.

- first three commits tidy up
- 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

Diff Detail

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

Event Timeline

linter introduced syntax errors

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

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

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

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...

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

Note to self: also backport #13048 after this

179 ↗(On Diff #9902)

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

This revision is now accepted and ready to land.Jul 3 2019, 13:40