Page MenuHomePhabricator

Label length unit tests for parse_name()
ClosedPublic

Authored by nakihito on Feb 25 2020, 21:25.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING9c34988e63da: Label length unit tests for parse_name()
rABC9c34988e63da: Label length unit tests for parse_name()
Summary

Adds some label length unit tests for the seeder's parse_name() function.

Depends on D4418

Test Plan
ninja check

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

nakihito created this revision.Feb 25 2020, 21:25
jasonbcox requested changes to this revision.Feb 25 2020, 22:55
jasonbcox added inline comments.
src/seeder/test/seeder_tests.cpp
117 ↗(On Diff #16502)

You can init this all at once using what's called a "fill constructor": std::string s6a (MAX_LABEL_LENGTH, 'x'); see http://www.cplusplus.com/reference/string/string/string/

This revision now requires changes to proceed.Feb 25 2020, 22:55
jasonbcox added inline comments.
src/seeder/test/seeder_tests.cpp
117 ↗(On Diff #16502)

This has the added benefit of making it immediately obvious how long the string is. Making it const is even better.

nakihito updated this revision to Diff 16510.Feb 25 2020, 23:01

Removed for-loop initialization.

jasonbcox requested changes to this revision.Feb 25 2020, 23:06
jasonbcox added inline comments.
src/seeder/test/seeder_tests.cpp
118 ↗(On Diff #16510)

you dont need this check anymore

This revision now requires changes to proceed.Feb 25 2020, 23:06
nakihito updated this revision to Diff 16512.Feb 25 2020, 23:12

Added const and removed unnecessary check.

jasonbcox accepted this revision.Feb 26 2020, 00:53
jasonbcox added inline comments.
src/seeder/test/seeder_tests.cpp
20 ↗(On Diff #16512)

Same feedback as this comment for a future diff: https://reviews.bitcoinabc.org/D4418#inline-32872

This revision is now accepted and ready to land.Feb 26 2020, 00:53
nakihito updated this revision to Diff 16630.Feb 28 2020, 21:48

Rebased.

This revision was automatically updated to reflect the committed changes.