Page MenuHomePhabricator

Add some unit tests for write_name() for seeder
ClosedPublic

Authored by nakihito on Thu, Mar 12, 18:08.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING3cac51b8198f: Add some unit tests for write_name() for seeder
rABC3cac51b8198f: Add some unit tests for write_name() for seeder
Summary

Adds some unit tests for write_name() in seeder/dns.cpp. This
function, like parse_name() is critical for dns operations and should
have some tests to increase confidence in correct behavior.

Test Plan
ninja check-bitcoin-seeder

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.Thu, Mar 12, 18:08
Owners added a reviewer: Restricted Owners Package.Thu, Mar 12, 18:08
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Mar 12, 18:08
deadalnix requested changes to this revision.Fri, Mar 13, 07:39
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/seeder/dns.h
44 ↗(On Diff #16903)

All these functions being pulled in the header but only used by the test probably indicate that there is an higher level API problem here. This is not meant to be consumed by the application, which indicate dns.cpp probably should be several modules instead of one.

Not something I expect you to fix in that patch, but you need to be thinking about it when you form plans to move forward with this.

src/seeder/test/dns_tests.cpp
267 ↗(On Diff #16903)

It seems like there is very little coupling between this and what comes before. that is a good indication that this should be two tests instead of one dns kitchen sink test.

This revision now requires changes to proceed.Fri, Mar 13, 07:39
nakihito planned changes to this revision.Fri, Mar 13, 23:21
nakihito updated this revision to Diff 17056.Thu, Mar 19, 19:16

Split dns_tests.cpp into write_name_tests.cpp and parse_name_tests.cpp, moved CreateDNSQuestionNameField() to dns_util.h.

deadalnix accepted this revision.Sat, Mar 21, 00:39
This revision is now accepted and ready to land.Sat, Mar 21, 00:39
This revision was automatically updated to reflect the committed changes.