Page MenuHomePhabricator

[tests] Fix checkinputs_test to work under magnetic anomaly
AbandonedPublic

Authored by jasonbcox on Sun, Nov 18, 20:01.

Details

Reviewers
deadalnix
schancel
Group Reviewers
Restricted Project
Summary

As per title. Add CTOR ordering to test block construction. Use
NULLDUMMY instead of CLEANSTACK for checking consensus vs non-consensus
script cache behavior.

Test Plan
make -j10 VERBOSE=1 check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
unit-test
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3972
Build 6016: Bitcoin ABC Teamcity Staging
Build 6015: arc lint + arc unit

Event Timeline

schancel created this revision.Sun, Nov 18, 20:01
Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Nov 18, 20:01
jasonbcox added inline comments.
src/test/test_bitcoin.cpp
170 ↗(On Diff #5897)

Is this comment still relevant? Either I'm missing something or it's already done above.

schancel updated this revision to Diff 5898.Sun, Nov 18, 21:51

Fix comments

This is a better approach toward fixing that test.

src/test/test_bitcoin.cpp
163 ↗(On Diff #5898)

I don't think this really warrant to modify the test framework.

src/test/txvalidationcache_tests.cpp
202 ↗(On Diff #5898)

It's the last item. You are pushing onto a stack so the item you push first end up being the last item on the stack.

schancel updated this revision to Diff 5899.Sun, Nov 18, 22:59

Fix tests with hackiness. Should try to use P2SH

schancel updated this revision to Diff 5903.Mon, Nov 19, 00:54

Fix multisig p2sh

schancel retitled this revision from WIP fix unit tests to [tests] Fix checkinputs_test to work under magnetic anomaly.Mon, Nov 19, 00:54
schancel updated this revision to Diff 5904.Mon, Nov 19, 00:57

Fix whitespace and unneeded includes

jasonbcox requested changes to this revision.Mon, Nov 19, 01:10
jasonbcox added inline comments.
src/test/txvalidationcache_tests.cpp
198 ↗(On Diff #5904)

Either comment is out of date, or this appears to be missing OP_1

This revision now requires changes to proceed.Mon, Nov 19, 01:10
jasonbcox added inline comments.Mon, Nov 19, 01:11
src/test/txvalidationcache_tests.cpp
198 ↗(On Diff #5904)

Oh I see, I think this comment needs to be moved above ^^

deadalnix requested changes to this revision.Mon, Nov 19, 01:12
deadalnix added inline comments.
src/script/interpreter.cpp
1084 ↗(On Diff #5903)

Revert

1088 ↗(On Diff #5903)

revert

src/test/txvalidationcache_tests.cpp
185 ↗(On Diff #5903)

There is no point in bringing the key store in there. You are building the script to begin with so anything you'll pull of the store you already have.

188 ↗(On Diff #5903)

I'm not sure why you want to do P2SH here instead of bare multisig. If there is a reason, it should be mentioned. if there is no reason, then don't do it.

200 ↗(On Diff #5903)

This would benefit being scoped to destruct the temporaries.

202 ↗(On Diff #5903)

This is not spending, this is mining.

243 ↗(On Diff #5903)

You can sign the exact same ways as before. You just need to push the non nulldummy element.

schancel updated this revision to Diff 5905.Mon, Nov 19, 01:12
schancel marked an inline comment as done.

Fix comments

schancel updated this revision to Diff 5906.Mon, Nov 19, 01:14

Removed unused junk var

jasonbcox added inline comments.Mon, Nov 19, 01:17
src/test/txvalidationcache_tests.cpp
243 ↗(On Diff #5903)

+1. doing this a different way is confusing

schancel marked an inline comment as done.Mon, Nov 19, 01:18
schancel added inline comments.
src/test/txvalidationcache_tests.cpp
243 ↗(On Diff #5903)

I did that

jasonbcox commandeered this revision.Mon, Nov 19, 07:01
jasonbcox abandoned this revision.
jasonbcox edited reviewers, added: schancel; removed: jasonbcox.