Page MenuHomePhabricator

Make CDnsSeedOpts unit testable
ClosedPublic

Authored by thonkle on Jan 7 2022, 19:18.

Details

Reviewers
Fabien
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABCbca5c5ee6ca6: Make CDnsSeedOpts unit testable
Summary

Move CDnsSeedOpts to its own files to make it more testable.
Only one basic proof of concept test was added to not clutter this patch.
Better test coverage will come with improved argument checking (example: D10748)

Test Plan
ninja check-seeder bitcoin-seeder

Sanity checks:

./src/seeder/bitcoin-seeder -host=seeder.status.cash -ns=localhost -mbox=thonkle@protonmail.com -port=5555
less dnsseed.dump

Diff Detail

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

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jan 7 2022, 19:18
thonkle requested review of this revision.Jan 7 2022, 19:18
Fabien requested changes to this revision.Jan 8 2022, 11:19
Fabien added a subscriber: Fabien.

Good overall, but a few nits needs to be addressed.

The new files names can be improved a bit, e.g. don't shorten options.h and rename the test file to be options_tests or something like that, so you can extend the tests to other methods of the same class in the future if you need it.

src/seeder/opts.cpp
7 ↗(On Diff #31703)

Nit: include first on its own line

src/seeder/opts.h
8 ↗(On Diff #31703)

Just forward declare ArgsManager instead

This revision now requires changes to proceed.Jan 8 2022, 11:19
  • Rename files so opts -> options
  • Fix #include nits
This revision is now accepted and ready to land.Jan 9 2022, 09:38
This revision was automatically updated to reflect the committed changes.