Page MenuHomePhabricator
Differential D935

Added missing test cases for SigHashType
ClosedPublic
Actions

Authored by jasonbcox on Jan 12 2018, 08:18.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rSTAGINGf2feb2421ab1: Added missing test cases for SigHashType
rABCf2feb2421ab1: Added missing test cases for SigHashType
Summary

Added missing tests according to Amaury's comments on D885.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jasonbcox created this revision.Jan 12 2018, 08:18
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 12 2018, 08:18
deadalnix requested changes to this revision.Jan 12 2018, 13:57
deadalnix added inline comments.
src/test/script_sighashtype_tests.cpp
139 ↗(On Diff #2462)

I think that'd be worth it to serialize and unsupported sighashtype to see it is not botched, as well as some combination of flags.

An option would be to just go

for (int i = 0; i < 256; i++) {
    (CDataStream(SER_DISK, 0) << SigHashType(i)) >>
    unserializedOutput;
    BOOST_CHECK_EQUAL(unserializedOutput, i);
}

That's not very subtle, but that do the job :)

This revision now requires changes to proceed.Jan 12 2018, 13:57
jasonbcox updated this revision to Diff 2465.Jan 12 2018, 17:24
jasonbcox marked an inline comment as done.

Better edge case test coverage

Harbormaster completed remote builds in B1584: Diff 2465.Jan 12 2018, 17:24
jasonbcox added inline comments.Jan 12 2018, 17:24
src/test/script_sighashtype_tests.cpp
139 ↗(On Diff #2462)

Good idea. I did something similar, but tested all flags for every value tested as well.

deadalnix accepted this revision.Jan 12 2018, 23:16
deadalnix added inline comments.
src/test/script_sighashtype_tests.cpp
132 ↗(On Diff #2465)

Do not put the comment on the same line as code.

This revision is now accepted and ready to land.Jan 12 2018, 23:16
Closed by commit rABCf2feb2421ab1: Added missing test cases for SigHashType (authored by jasonbcox). · Explain WhyJan 13 2018, 00:30
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
test/
27 lines

Diff 2477

View Options

src/test/script_sighashtype_tests.cpp

Loading...