The internal/notify package has 11.1% test coverage. Only the URL validation functions (ValidateWebhookURL, IsAllowedScheme) are tested. The actual notification delivery logic is untested.
Notifications are the primary output of dnswatcher. If notifications are malformed, lost, or sent to wrong endpoints, the daemon provides no value. The notification code is also where goroutines are spawned without tracking (fire-and-forget), making it especially important to verify correctness.
Recommendation
Add tests using httptest.Server or a mock http.RoundTripper to verify:
Correct ntfy headers (Title, Priority) and body
Correct Slack/Mattermost JSON payload structure
Priority mapping for all priority levels
Error handling for non-2xx responses
Behavior when no webhooks are configured
The existing transport field on Service already supports dependency injection for testing.
Category
Should-fix before 1.0.
## Problem
The `internal/notify` package has **11.1% test coverage**. Only the URL validation functions (`ValidateWebhookURL`, `IsAllowedScheme`) are tested. The actual notification delivery logic is untested.
## What is Untested
- `sendNtfy()` — ntfy notification delivery, header formatting, priority mapping
- `sendSlack()` — Slack/Mattermost webhook payload construction, JSON marshaling, color mapping
- `SendNotification()` — goroutine dispatch to all configured endpoints
- HTTP error handling (non-2xx responses, network errors, timeouts)
- `newRequest()` — HTTP request construction from pre-validated URL
- Priority mapping functions (`ntfyPriority`, `slackColor`)
## Why This Matters
Notifications are the primary output of dnswatcher. If notifications are malformed, lost, or sent to wrong endpoints, the daemon provides no value. The notification code is also where goroutines are spawned without tracking (fire-and-forget), making it especially important to verify correctness.
## Recommendation
Add tests using `httptest.Server` or a mock `http.RoundTripper` to verify:
1. Correct ntfy headers (Title, Priority) and body
2. Correct Slack/Mattermost JSON payload structure
3. Priority mapping for all priority levels
4. Error handling for non-2xx responses
5. Behavior when no webhooks are configured
The existing `transport` field on Service already supports dependency injection for testing.
## Category
Should-fix before 1.0.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
The
internal/notifypackage has 11.1% test coverage. Only the URL validation functions (ValidateWebhookURL,IsAllowedScheme) are tested. The actual notification delivery logic is untested.What is Untested
sendNtfy()— ntfy notification delivery, header formatting, priority mappingsendSlack()— Slack/Mattermost webhook payload construction, JSON marshaling, color mappingSendNotification()— goroutine dispatch to all configured endpointsnewRequest()— HTTP request construction from pre-validated URLntfyPriority,slackColor)Why This Matters
Notifications are the primary output of dnswatcher. If notifications are malformed, lost, or sent to wrong endpoints, the daemon provides no value. The notification code is also where goroutines are spawned without tracking (fire-and-forget), making it especially important to verify correctness.
Recommendation
Add tests using
httptest.Serveror a mockhttp.RoundTripperto verify:The existing
transportfield on Service already supports dependency injection for testing.Category
Should-fix before 1.0.
Closing: completed by PR #79 (merged).