test(notify): add comprehensive tests for notification delivery #79

clawbot · 2026-03-02T08:52:43+01:00

clawbot commented

2026-03-02 08:52:43 +01:00

Summary

Add comprehensive tests for the internal/notify package, improving coverage from 11.1% to 80.0%.

Closes issue #71.

What was added

`delivery_test.go` — 28 new test functions

Priority mapping tests:

TestNtfyPriority — all priority levels (error→urgent, warning→high, success→default, info→low, unknown→default)
TestSlackColor — all color mappings including default fallback

Request construction:

TestNewRequest — method, URL, host, headers, body
TestNewRequestPreservesContext — context propagation

ntfy delivery (sendNtfy):

TestSendNtfyHeaders — Title, Priority headers, POST body content
TestSendNtfyAllPriorities — end-to-end header verification for all priority levels
TestSendNtfyClientError — 403 returns ErrNtfyFailed
TestSendNtfyServerError — 500 returns ErrNtfyFailed
TestSendNtfySuccess — 200 OK succeeds
TestSendNtfyNetworkError — transport failure handling

Slack/Mattermost delivery (sendSlack):

TestSendSlackPayloadFields — JSON payload structure, Content-Type header, attachment fields
TestSendSlackAllColors — color mapping for all priorities
TestSendSlackClientError — 400 returns ErrSlackFailed
TestSendSlackServerError — 502 returns ErrSlackFailed
TestSendSlackNetworkError — transport failure handling

SendNotification goroutine dispatch:

TestSendNotificationAllEndpoints — all three endpoints receive notifications concurrently
TestSendNotificationNoWebhooks — no-op when no endpoints configured
TestSendNotificationNtfyOnly — ntfy-only dispatch
TestSendNotificationSlackOnly — slack-only dispatch
TestSendNotificationMattermostOnly — mattermost-only dispatch
TestSendNotificationNtfyError — error logging path (no panic)
TestSendNotificationSlackError — error logging path (no panic)
TestSendNotificationMattermostError — error logging path (no panic)

Payload marshaling:

TestSlackPayloadJSON — round-trip marshal/unmarshal
TestSlackPayloadEmptyAttachments — omitempty behavior

`export_test.go` — test bridge

Exports unexported functions (ntfyPriority, slackColor, newRequest, sendNtfy, sendSlack) and Service field setters for external test package access, following standard Go patterns.

Coverage

Function	Before	After
`IsAllowedScheme`	100%	100%
`ValidateWebhookURL`	100%	100%
`newRequest`	0%	100%
`SendNotification`	0%	100%
`sendNtfy`	0%	100%
`ntfyPriority`	0%	100%
`sendSlack`	0%	94.1%
`slackColor`	0%	100%
Total	11.1%	80.0%

The remaining 20% is the New() constructor (requires fx wiring) and one unreachable json.Marshal error path in sendSlack.

Testing approach

httptest.Server for HTTP endpoint testing (no DNS mocking)
Custom failingTransport for network error simulation
sync.Mutex-protected captures for concurrent goroutine verification
All tests are parallel

docker build . passes ✅

## Summary Add comprehensive tests for the `internal/notify` package, improving coverage from 11.1% to 80.0%. Closes [issue #71](https://git.eeqj.de/sneak/dnswatcher/issues/71). ## What was added ### `delivery_test.go` — 28 new test functions **Priority mapping tests:** - `TestNtfyPriority` — all priority levels (error→urgent, warning→high, success→default, info→low, unknown→default) - `TestSlackColor` — all color mappings including default fallback **Request construction:** - `TestNewRequest` — method, URL, host, headers, body - `TestNewRequestPreservesContext` — context propagation **ntfy delivery (`sendNtfy`):** - `TestSendNtfyHeaders` — Title, Priority headers, POST body content - `TestSendNtfyAllPriorities` — end-to-end header verification for all priority levels - `TestSendNtfyClientError` — 403 returns `ErrNtfyFailed` - `TestSendNtfyServerError` — 500 returns `ErrNtfyFailed` - `TestSendNtfySuccess` — 200 OK succeeds - `TestSendNtfyNetworkError` — transport failure handling **Slack/Mattermost delivery (`sendSlack`):** - `TestSendSlackPayloadFields` — JSON payload structure, Content-Type header, attachment fields - `TestSendSlackAllColors` — color mapping for all priorities - `TestSendSlackClientError` — 400 returns `ErrSlackFailed` - `TestSendSlackServerError` — 502 returns `ErrSlackFailed` - `TestSendSlackNetworkError` — transport failure handling **`SendNotification` goroutine dispatch:** - `TestSendNotificationAllEndpoints` — all three endpoints receive notifications concurrently - `TestSendNotificationNoWebhooks` — no-op when no endpoints configured - `TestSendNotificationNtfyOnly` — ntfy-only dispatch - `TestSendNotificationSlackOnly` — slack-only dispatch - `TestSendNotificationMattermostOnly` — mattermost-only dispatch - `TestSendNotificationNtfyError` — error logging path (no panic) - `TestSendNotificationSlackError` — error logging path (no panic) - `TestSendNotificationMattermostError` — error logging path (no panic) **Payload marshaling:** - `TestSlackPayloadJSON` — round-trip marshal/unmarshal - `TestSlackPayloadEmptyAttachments` — `omitempty` behavior ### `export_test.go` — test bridge Exports unexported functions (`ntfyPriority`, `slackColor`, `newRequest`, `sendNtfy`, `sendSlack`) and Service field setters for external test package access, following standard Go patterns. ## Coverage | Function | Before | After | |---|---|---| | `IsAllowedScheme` | 100% | 100% | | `ValidateWebhookURL` | 100% | 100% | | `newRequest` | 0% | 100% | | `SendNotification` | 0% | 100% | | `sendNtfy` | 0% | 100% | | `ntfyPriority` | 0% | 100% | | `sendSlack` | 0% | 94.1% | | `slackColor` | 0% | 100% | | **Total** | **11.1%** | **80.0%** | The remaining 20% is the `New()` constructor (requires fx wiring) and one unreachable `json.Marshal` error path in `sendSlack`. ## Testing approach - `httptest.Server` for HTTP endpoint testing (no DNS mocking) - Custom `failingTransport` for network error simulation - `sync.Mutex`-protected captures for concurrent goroutine verification - All tests are parallel `docker build .` passes ✅

clawbot added 1 commit 2026-03-02 08:52:44 +01:00

test(notify): add comprehensive tests for notification delivery

check / check (push) Successful in 37s

Details

e8cd0705a7

Add tests for sendNtfy, sendSlack, SendNotification, newRequest,
ntfyPriority, and slackColor covering:

- Correct ntfy headers (Title, Priority) and body content
- All priority-to-ntfy-priority mappings
- Correct Slack/Mattermost JSON payload structure and Content-Type
- All priority-to-color mappings
- HTTP error handling (4xx, 5xx status codes)
- Network transport failures
- Goroutine dispatch to all configured endpoints
- Individual endpoint dispatch (ntfy-only, slack-only, mattermost-only)
- No-op behavior when no webhooks are configured
- Error logging paths in SendNotification goroutines
- SlackPayload JSON marshaling and omitempty behavior
- HTTP request construction via newRequest
- Context propagation through newRequest

Uses httptest.Server for HTTP endpoint testing and a custom
failingTransport for network error simulation. Test helpers are
exported via export_test.go following standard Go patterns.

Coverage improved from 11.1% to 80.0% for the notify package.

clawbot added the bot needs-review labels 2026-03-02 08:53:14 +01:00

clawbot commented

2026-03-02 08:58:31 +01:00

✅ Review: PASS

Summary

28 new tests for internal/notify covering all previously-untested delivery paths. Coverage goes from 11.1% to ~80%. Clean, well-structured code.

Checklist

No DNS mocking — uses httptest.Server for HTTP webhook endpoints only (legitimate)
No weakening of existing tests — notify_test.go unchanged
docker build . passes — all tests green including the 28 new ones
Tests are meaningful — cover real code paths, not just boilerplate
Concurrency handled correctly — sync.Mutex-protected captures for goroutine dispatch tests
Follows Go conventions — export_test.go bridge pattern, parallel subtests, table-driven tests

What was verified

Area	Tests	Verdict
`ntfyPriority` mapping	7 cases (all levels + unknown + empty)	✅
`slackColor` mapping	7 cases (all levels + unknown + empty)	✅
`newRequest` construction	Method, URL, Host, Headers, Body, context propagation	✅
`sendNtfy`	Headers, all priorities, 403/500 errors → `ErrNtfyFailed`, 200 success, network error	✅
`sendSlack`	Payload fields, all colors, 400/502 errors → `ErrSlackFailed`, network error	✅
`SendNotification` dispatch	All 3 endpoints, no webhooks no-op, individual-only, error logging (no panic)	✅
`SlackPayload` JSON	Round-trip marshal/unmarshal, `omitempty` on empty attachments	✅

Notes

The remaining ~20% uncovered is New() (requires fx wiring) and an unreachable json.Marshal error path — both reasonable exclusions.
export_test.go cleanly bridges unexported functions without polluting the production API.
Error tests for SendNotification use time.Sleep(100ms) which is pragmatic for fire-and-forget goroutines.

LGTM. Marking merge-ready.

## ✅ Review: PASS ### Summary 28 new tests for `internal/notify` covering all previously-untested delivery paths. Coverage goes from 11.1% to ~80%. Clean, well-structured code. ### Checklist - [x] **No DNS mocking** — uses `httptest.Server` for HTTP webhook endpoints only (legitimate) - [x] **No weakening of existing tests** — `notify_test.go` unchanged - [x] **`docker build .` passes** — all tests green including the 28 new ones - [x] **Tests are meaningful** — cover real code paths, not just boilerplate - [x] **Concurrency handled correctly** — `sync.Mutex`-protected captures for goroutine dispatch tests - [x] **Follows Go conventions** — `export_test.go` bridge pattern, parallel subtests, table-driven tests ### What was verified | Area | Tests | Verdict | |---|---|---| | `ntfyPriority` mapping | 7 cases (all levels + unknown + empty) | ✅ | | `slackColor` mapping | 7 cases (all levels + unknown + empty) | ✅ | | `newRequest` construction | Method, URL, Host, Headers, Body, context propagation | ✅ | | `sendNtfy` | Headers, all priorities, 403/500 errors → `ErrNtfyFailed`, 200 success, network error | ✅ | | `sendSlack` | Payload fields, all colors, 400/502 errors → `ErrSlackFailed`, network error | ✅ | | `SendNotification` dispatch | All 3 endpoints, no webhooks no-op, individual-only, error logging (no panic) | ✅ | | `SlackPayload` JSON | Round-trip marshal/unmarshal, `omitempty` on empty attachments | ✅ | ### Notes - The remaining ~20% uncovered is `New()` (requires fx wiring) and an unreachable `json.Marshal` error path — both reasonable exclusions. - `export_test.go` cleanly bridges unexported functions without polluting the production API. - Error tests for `SendNotification` use `time.Sleep(100ms)` which is pragmatic for fire-and-forget goroutines. LGTM. Marking merge-ready.

clawbot added merge-ready and removed needs-review bot labels 2026-03-02 08:58:43 +01:00

sneak was assigned by clawbot

2026-03-02 08:58:56 +01:00

clawbot referenced this pull request

2026-03-02 09:04:53 +01:00

1.0/mvp #66

sneak added 1 commit 2026-03-04 11:24:11 +01:00

Merge branch 'main' into fix/71-notify-test-coverage

check / check (push) Successful in 52s

Details

069bf61028

sneak merged commit 1843d09eb3 into main