Paths

Table of Contentst

Differential D2085

[qa] fix p2p-fullblocktest.py to support magnetic anomaly
AbandonedPublic
Actions

Authored by jasonbcox on Nov 18 2018, 08:33.

Details

Reviewers

Fabien
schancel
deadalnix

Group Reviewers

Restricted Project

Summary

Per title, failures associated with the activation of CTOR, CLEANSTACK,
and a change in error message introduced in D2076

Test Plan

make check && ./test/functional/test_runner.py p2p-fullblocktest

Diff Detail

Repository

rABC Bitcoin ABC

Branch

p2p-fullblocktest

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 3950
Build 5972: Bitcoin ABC Buildbot (legacy)
Build 5971: arc lint + arc unit

Event Timeline

schancel created this revision.Nov 18 2018, 08:33

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 18 2018, 08:33

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

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

Harbormaster failed remote builds in B3950: Diff 5879!Nov 18 2018, 08:41

Fix last test

schancel retitled this revision from [qa] WIP fix p2p-fullblocktest.py to support magnetic anomaly to [qa] fix p2p-fullblocktest.py to support magnetic anomaly.Nov 18 2018, 08:51

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

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

Harbormaster failed remote builds in B3951: Diff 5880!Nov 18 2018, 08:58

Fabien requested changes to this revision.Nov 18 2018, 10:11

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/validation.cpp
3355 ↗	(On Diff #5880)	bad-txns-duplicate is already raised on validation.cpp:3122, better change the error name to avoid confusion. From my understanding, the 2 errors are slightly different.
test/functional/test_framework/blocktools.py
31 ↗	(On Diff #5880)	Good idea !

This revision now requires changes to proceed.Nov 18 2018, 10:11

Fabien added a child revision: D2086: [qa] fix abc-p2p-fullblock test to support magnetic anomaly.Nov 18 2018, 10:16

jasonbcox requested changes to this revision.Nov 18 2018, 15:15

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

test/functional/p2p-fullblocktest.py
561 ↗	(On Diff #5880)	numTxes -> numTxs
565 ↗	(On Diff #5880)	can be simplified to `for i in range(numTxs):`
613 ↗	(On Diff #5880)	Why the comment if it's accepted?

jasonbcox added inline comments.Nov 18 2018, 16:25

src/validation.cpp
3355 ↗	(On Diff #5880)	This doesn't look related to making the test green. Please focus on that first.

jasonbcox added inline comments.Nov 18 2018, 16:32

test/functional/p2p-fullblocktest.py
565 ↗	(On Diff #5880)	nvm

jasonbcox commandeered this revision.Nov 18 2018, 16:33

jasonbcox edited reviewers, added: schancel; removed: jasonbcox.

Fixed according to feedback

jasonbcox marked an inline comment as done.Nov 18 2018, 17:06

jasonbcox added inline comments.

test/functional/p2p-fullblocktest.py
1311 ↗	(On Diff #5887)	This looks wrong, but passes. I don't know why yet.

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

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

Harbormaster failed remote builds in B3957: Diff 5887!Nov 18 2018, 17:13

jasonbcox removed a child revision: D2086: [qa] fix abc-p2p-fullblock test to support magnetic anomaly.Nov 18 2018, 17:39

deadalnix requested changes to this revision.Nov 18 2018, 18:28

deadalnix added a subscriber: deadalnix.

deadalnix added inline comments.

test/functional/p2p-fullblocktest.py
561 ↗	(On Diff #5887)	Ok this is the worst name ever. This is not necessarily creating a long chain, nor it is the goal at all, because we are testing sigops.
1041 ↗	(On Diff #5887)	When you see things like this, you should move the comment on the previous line so that formatting isn't screwed up.

This revision now requires changes to proceed.Nov 18 2018, 18:28

Feedback

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

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

Harbormaster failed remote builds in B3962: Diff 5894!Nov 18 2018, 18:57

deadalnix added inline comments.Nov 18 2018, 19:02

test/functional/p2p-fullblocktest.py
608 ↗	(On Diff #5894)	This changes completely what this test is doing, why ?
793 ↗	(On Diff #5887)	Revert

schancel added inline comments.Nov 18 2018, 19:11

src/validation.cpp
3355 ↗	(On Diff #5880)	This is a change in the error behavior. It changes the external API.
test/functional/p2p-fullblocktest.py
608 ↗	(On Diff #5894)	No. It does the same thing AFAICS. The old version was invalid because of LTOR changing transaction indices in the block. So, I change it to generate the same thing twice.