Page MenuHomePhabricator

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

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

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC0890051f4081: Add new test cases to abc-finalize-.py
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

Remove assertions against block height and rebase

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

Rebase

deadalnix requested changes to this revision.Sun, Dec 2, 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.Sun, Dec 2, 22:31
Fabien updated this revision to Diff 6277.Tue, Dec 4, 16:23

Update as per review

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

Rebase

deadalnix requested changes to this revision.Thu, Dec 6, 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.Thu, Dec 6, 18:06
Fabien marked an inline comment as done.Thu, Dec 6, 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.Thu, Dec 6, 19:43

Address feedbacks

deadalnix accepted this revision.Thu, Dec 6, 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.Thu, Dec 6, 20:02
Fabien updated this revision to Diff 6287.Thu, Dec 6, 20:17

Update assertion

This revision was automatically updated to reflect the committed changes.