Page MenuHomePhabricator

Add new test cases to abc-finalize-.py
ClosedPublic

Authored by Fabien on Nov 30 2018, 17:41.

Details

Summary

Add new test cases and assertions.
Also replace the sync_blocks calls with a proper wait_for_tip() function.

Depends on D2161

Test Plan
./test/functional/test_runner.py abc-finalize-block

Diff Detail

Repository
rABC Bitcoin ABC
Branch
add_finalization_test_cases
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4274
Build 6613: Bitcoin ABC Teamcity Staging
Build 6612: arc lint + arc unit

Event Timeline

Fabien created this revision.Nov 30 2018, 17:41
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 30 2018, 17:41
Herald added a subscriber: schancel. · View Herald Transcript
Fabien updated this revision to Diff 6208.Nov 30 2018, 21:01

Remove assertions against block height and rebase

Fabien added a child revision: Restricted Differential Revision.Nov 30 2018, 22:16
Fabien updated this revision to Diff 6233.Dec 2 2018, 21:43

Rebase

deadalnix requested changes to this revision.Dec 2 2018, 22:31

This ends up changing all the tests for no good reasons. Just add the new tests cases after the existing ones. You can use reconsiderblock/invalidate block to recreate the situation you desire.

test/functional/abc-finalize-block.py
154 ↗(On Diff #6233)

It's clearly not doing this anymore.

171 ↗(On Diff #6233)
alt_node.generate(10)

And then wait for node to catch up. You want to check that the node sync the way you expect note force the test framework to do it.

This revision now requires changes to proceed.Dec 2 2018, 22:31
Fabien updated this revision to Diff 6277.Dec 4 2018, 16:23

Update as per review

Fabien edited the summary of this revision. (Show Details)Dec 4 2018, 16:28
Fabien updated this revision to Diff 6280.Dec 4 2018, 17:30

Rebase

deadalnix requested changes to this revision.Dec 6 2018, 18:06
deadalnix added inline comments.
test/functional/abc-finalize-block.py
152 ↗(On Diff #6280)

likestamp

171 ↗(On Diff #6280)

210 is finalized, so why change the comment ?

179 ↗(On Diff #6280)

This test really isn't testing anything useful. I don't see in what situation you'd end finalizing that block anyways. Plus we already checked that the finalized block is the tip.

180 ↗(On Diff #6280)

Plus you actually check the value here.

This revision now requires changes to proceed.Dec 6 2018, 18:06
Fabien marked an inline comment as done.Dec 6 2018, 19:37
Fabien added inline comments.
test/functional/abc-finalize-block.py
171 ↗(On Diff #6280)

Good catch, looks like a merge error while rebasing.

Fabien updated this revision to Diff 6286.Dec 6 2018, 19:43

Address feedbacks

deadalnix accepted this revision.Dec 6 2018, 20:02
deadalnix added inline comments.
test/functional/abc-finalize-block.py
228 ↗(On Diff #6286)

Maybe using alt_node_new_tip here would be best.

This revision is now accepted and ready to land.Dec 6 2018, 20:02
Fabien updated this revision to Diff 6287.Dec 6 2018, 20:17

Update assertion

This revision was automatically updated to reflect the committed changes.