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 17841 Build 35508: Build Diff lint-circular-dependencies · build-without-wallet · build-diff · build-clang-tidy · build-debug · build-clang Build 35507: 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 | Nit: include first on its own line | |
| src/seeder/opts.h | ||
| 8 | Just forward declare ArgsManager instead | |