Page MenuHomePhabricator

Added missing test cases for SigHashType
ClosedPublic

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

Details

Summary

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

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
master
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1581
Build 1581: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Jan 12 2018, 13:57
deadalnix added inline comments.
src/test/script_sighashtype_tests.cpp
139

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 marked an inline comment as done.

Better edge case test coverage

src/test/script_sighashtype_tests.cpp
139

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

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
This revision was automatically updated to reflect the committed changes.