#36229 test: Cover AddrMan add edge cases

full analysis

https://github.com/bitcoin/bitcoin/pull/36229 · mertsaner · +164/-0 in 1 files, 1 commits · labels: Tests

Goal

  • Cover address manager addition edge cases with unit tests
  • Ensures subtle peer-address handling behaviors remain intact during refactoring

This pull request adds several unit test cases to src/test/addrman_tests.cpp exercising AddrMan::Add edge cases. It verifies behavior around empty, unroutable, and duplicate batches, timestamp clamping and boundary updates, service flag preservation, and bucket and tried-table retention.

Problem: AddrMan address addition edge cases lack explicit unit test assertions, making it harder to verify that subtle behaviors remain unchanged during refactoring.

Category: P2P (#63 of 65)

P4 · test coverage

  • P4 because it tests existing address manager behavior without fixing a bug or regression
  • Provides marginal impact since it does not expand measured test coverage metrics

P4 because it adds unit test coverage for existing address management behaviors without fixing any bug, preventing a demonstrated regression, or expanding test coverage metrics. Reviewer brunoerg highlighted that CoreCheck reported no gained coverage.

Membership: Tests AddrMan, the address manager responsible for tracking and selecting network peers.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0

Reviewability: Ready

  • Ready to review
  • Small unit test patch with no blockers

The patch is small, cleanly applying unit tests, with no mechanical blockers or requested reworks pending.

Author status: active; force-pushed updates but has not yet replied to the reviewer comment.

Open concerns:

  • brunoerg noted that CoreCheck showed no gained coverage and asked what practical edge cases or mutations the tests address.

Agreement: Crickets

  • No substantive reviews or concept approvals yet
  • Questions whether the tests add coverage or address practical mutations (brunoerg)

brunoerg asked for the motivation and coverage impact of the test cases; no reviews yet

No substantive reviews or Concept ACKs have been posted, and the single reviewer inquiry asks about motivation rather than raising an architectural objection.

  • brunoerg asked: 'corecheck doesn't show any gained coverage, can you tell us what these edge cases are addressing in practice? any mutation?'

Review verdicts (DrahtBot): 0

Files

164 lines under test/bench/ci.

  • src/test/addrman_tests.cpp +164/-0

Card

This pull request adds unit tests for AddrMan::Add edge cases such as empty or invalid batches, timestamp penalty clamping, and bucket retention across updates. It aims to pin down existing address management behavior without modifying production code. The change is marginal since it fixes no bug and, as noted in review, introduces no new coverage in automated tooling. The PR is ready for review with no dependencies, though the author has yet to answer a question regarding the practical motivation for the tests.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:23, confidence high, input hash c11a1540570a7af3