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)
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project - Commits
- rABCbca5c5ee6ca6: Make CDnsSeedOpts unit testable
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
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- seeder-opts-tests
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 17847 Build 35520: Build Diff build-debug · build-diff · build-without-wallet · lint-circular-dependencies · build-clang-tidy · build-clang Build 35519: arc lint + arc unit
Event Timeline
Comment Actions
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 |