Skip to content

test(server): cover Healthz notify path (MonitorHealthChan/notifyHealth) - #112

Merged
bithckr merged 3 commits into
mainfrom
miotte-pr10
Jun 10, 2026
Merged

test(server): cover Healthz notify path (MonitorHealthChan/notifyHealth)#112
bithckr merged 3 commits into
mainfrom
miotte-pr10

Conversation

@miotte

@miotte miotte commented May 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #110

Adds internal unit tests for the Healthz notify path: notifyHealth receiver registration/replacement and MonitorHealthChan fan-out + clean exit on channel close. Test-only, no production change.

Add internal unit tests for the health-signal broadcast path:

- notifyHealth registers a receiver and closes/replaces a prior
  receiver registered for the same client address.
- MonitorHealthChan fans a health code out to all registered
  notifiers and exits cleanly when its source channel is closed.

These were previously at 40% (notifyHealth) and 0% (MonitorHealthChan)
coverage; both are now fully covered. Test-only; no production change.

Fixes #110

Signed-off-by: Michael Otteni <MichaelGOtteni@gmail.com>
The fan-out test registered only one notifier, so it never exercised the
loop over the notifier registry with more than one entry. Register two
notifiers and assert both receive the broadcast code.

Signed-off-by: Michael Otteni <MichaelGOtteni@gmail.com>
Comment thread internal/server/health_internal_test.go Outdated
The standalone registration test duplicated the replace test's setup
without adding coverage. Fold it in by asserting the first notifier is
registered before the second replaces it, which also makes the replace
test's precondition explicit.

Addresses review feedback on #112.

Signed-off-by: Michael Otteni <MichaelGOtteni@gmail.com>
@rsperl

rsperl commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

@miotte - one more thing -- we discussed the need for having PR descriptions pretty short to keep the changelog manageable. I think the desc above could leave off the bullet points and below. Anything that's needed to explain the PR should be moved over to the ticket.

@bithckr
bithckr merged commit d637ca4 into main Jun 10, 2026
4 checks passed
@bithckr
bithckr deleted the miotte-pr10 branch June 10, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit coverage for Healthz notify path (MonitorHealthChan/notifyHealth)

3 participants