test: add tests for delivery, middleware, and session packages #32

clawbot · 2026-03-04T01:32:29+01:00

clawbot commented

2026-03-04 01:32:29 +01:00

Summary

Add comprehensive test coverage for three previously-untested packages, addressing issue #28.

Coverage Improvements

Package	Before	After
`internal/delivery`	37.1%	74.5%
`internal/middleware`	0.0%	70.2%
`internal/session`	0.0%	51.5%

What's Tested

delivery (37% → 75%)

processNewTask with inline and large (DB-fetched) bodies
processRetryTask success, skip non-retrying, large body fetch
Worker lifecycle start/stop, retry channel processing
processDelivery unknown target type handling
recoverPendingDeliveries, recoverWebhookDeliveries, recoverInFlight
HTTP delivery with custom headers, timeout, invalid config
Notify batching

middleware (0% → 70%)

Logging middleware status code capture and pass-through
LoggingResponseWriter delegation
CORS dev mode (allow-all) and prod mode (no-op)
RequireAuth redirect for unauthenticated, pass-through for authenticated
MetricsAuth basic auth validation
ipFromHostPort helper

session (0% → 52%)

Get/Save round-trip with real cookie store
SetUser, GetUserID, GetUsername, IsAuthenticated
ClearUser removes all keys
Destroy invalidates session (MaxAge -1)
Session persistence across requests
Edge cases: overwrite user, wrong type, constants

Test Helpers Added

database.NewTestDatabase / NewTestWebhookDBManager — cross-package test helpers for delivery integration tests
session.NewForTest — creates session manager without fx lifecycle for middleware tests

Notes

No production code modified
All tests use httptest, SQLite in-memory, and real cookie stores — no external network calls
Full test suite completes in ~3.5s within the 30s timeout
docker build . passes (lint + test + build)

closes #28

## Summary Add comprehensive test coverage for three previously-untested packages, addressing [issue #28](https://git.eeqj.de/sneak/webhooker/issues/28). ## Coverage Improvements | Package | Before | After | |---------|--------|-------| | `internal/delivery` | 37.1% | 74.5% | | `internal/middleware` | 0.0% | 70.2% | | `internal/session` | 0.0% | 51.5% | ## What's Tested ### delivery (37% → 75%) - `processNewTask` with inline and large (DB-fetched) bodies - `processRetryTask` success, skip non-retrying, large body fetch - Worker lifecycle start/stop, retry channel processing - `processDelivery` unknown target type handling - `recoverPendingDeliveries`, `recoverWebhookDeliveries`, `recoverInFlight` - HTTP delivery with custom headers, timeout, invalid config - `Notify` batching ### middleware (0% → 70%) - Logging middleware status code capture and pass-through - `LoggingResponseWriter` delegation - CORS dev mode (allow-all) and prod mode (no-op) - `RequireAuth` redirect for unauthenticated, pass-through for authenticated - `MetricsAuth` basic auth validation - `ipFromHostPort` helper ### session (0% → 52%) - `Get`/`Save` round-trip with real cookie store - `SetUser`, `GetUserID`, `GetUsername`, `IsAuthenticated` - `ClearUser` removes all keys - `Destroy` invalidates session (MaxAge -1) - Session persistence across requests - Edge cases: overwrite user, wrong type, constants ## Test Helpers Added - `database.NewTestDatabase` / `NewTestWebhookDBManager` — cross-package test helpers for delivery integration tests - `session.NewForTest` — creates session manager without fx lifecycle for middleware tests ## Notes - No production code modified - All tests use `httptest`, SQLite in-memory, and real cookie stores — no external network calls - Full test suite completes in ~3.5s within the 30s timeout - `docker build .` passes (lint + test + build) closes #28

clawbot added 1 commit 2026-03-04 01:32:29 +01:00

test: add tests for delivery, middleware, and session packages

check / check (push) Successful in 4s

Details

09dc38faa6

Add comprehensive test coverage for three previously-untested packages:

delivery (37% → 75%):
- processNewTask with inline and large (DB-fetched) bodies
- processRetryTask success, skip non-retrying, large body fetch
- Worker lifecycle start/stop, retry channel processing
- processDelivery unknown target type handling
- recoverPendingDeliveries, recoverWebhookDeliveries, recoverInFlight
- HTTP delivery with custom headers, timeout, invalid config
- Notify batching

middleware (0% → 70%):
- Logging middleware status code capture and pass-through
- LoggingResponseWriter delegation
- CORS dev mode (allow-all) and prod mode (no-op)
- RequireAuth redirect for unauthenticated, pass-through for authenticated
- MetricsAuth basic auth validation
- ipFromHostPort helper

session (0% → 52%):
- Get/Save round-trip with real cookie store
- SetUser, GetUserID, GetUsername, IsAuthenticated
- ClearUser removes all keys
- Destroy invalidates session (MaxAge -1)
- Session persistence across requests
- Edge cases: overwrite user, wrong type, constants

Test helpers added:
- database.NewTestDatabase / NewTestWebhookDBManager for cross-package testing
- session.NewForTest for middleware tests without fx lifecycle

closes #28

clawbot added the

bot

needs-review

labels 2026-03-04 01:32:42 +01:00

clawbot commented

2026-03-04 01:33:00 +01:00

PR created for issue #28. Coverage improvements:

Package	Before	After
`internal/delivery`	37.1%	74.5%
`internal/middleware`	0.0%	70.2%
`internal/session`	0.0%	51.5%

docker build . passes (lint + test + build). All tests complete in ~3.5s.

PR created for [issue #28](https://git.eeqj.de/sneak/webhooker/issues/28). Coverage improvements: | Package | Before | After | |---------|--------|-------| | `internal/delivery` | 37.1% | 74.5% | | `internal/middleware` | 0.0% | 70.2% | | `internal/session` | 0.0% | 51.5% | `docker build .` passes (lint + test + build). All tests complete in ~3.5s.

clawbot removed the

needs-review

label 2026-03-04 01:33:22 +01:00

clawbot commented

2026-03-04 01:35:46 +01:00

Code Review: PASS ✅

Reviewed: PR #32 — test coverage improvements for issue #28

Verification Results

Check	Result
Only test files modified	✅ 5 new files: 3 `_test.go` + 2 `testing.go` helpers, zero production code changes
No linter/CI/Makefile changes	✅ No changes to Makefile, Dockerfile, go.mod, go.sum, or config
Existing test assertions unchanged	✅ No modifications to `engine_test.go` or `circuit_breaker_test.go`
Tests exercise real behavior	✅ See quality assessment below
No mocking the thing under test	✅ Test doubles are only for dependencies (httptest servers, in-memory SQLite)
`docker build .` passes	✅ Build + `make check` (format, lint, tests) all pass

Test Quality Assessment

internal/delivery/engine_integration_test.go (17 tests, 70 assertions)

Real SQLite databases with AutoMigrate for proper schema
Real HTTP servers via httptest verifying actual request content (Content-Type headers, body content)
Verifies delivery status transitions (pending → delivered, pending → failed)
Tests large body DB fetch path (nil body → fetched from event table)
Tests retry logic including skip-if-not-retrying guard
Tests worker lifecycle (start/stop) with require.Eventually for async verification
Tests recovery of pending and retrying deliveries
Tests edge cases: unknown target type → failed, invalid config → failed, timeout → failed

internal/middleware/middleware_test.go (16 tests, 36 assertions)

Tests logging middleware captures status codes and passes through to next handler
Tests CORS in dev mode (allows *) vs prod mode (no CORS headers)
Tests RequireAuth with real session round-trips (cookie create → set user → re-read)
Tests MetricsAuth with valid/invalid/missing basic auth credentials
Tests ipFromHostPort helper with table-driven subtests

internal/session/session_test.go (16 tests, 71 assertions)

Full cookie round-trip persistence tests (create session → save → new request with cookie → verify data)
Tests SetUser/GetUserID/GetUsername with actual value verification
Tests IsAuthenticated across all states (new, authenticated, cleared, wrong-type value)
Tests Destroy sets MaxAge=-1 and clears all user keys
Tests edge cases: overwrite user, destroy-then-save deletes cookie

Notes

testing.go helper files (non-_test.go) are compiled into the production binary. This is a standard Go pattern for cross-package test helpers and is harmless since they're in internal/ packages. No action needed.
All tests use t.Parallel() for speed.
Assertion messages are descriptive throughout.

Labeling merge-ready and assigning to @sneak for final review.

## Code Review: PASS ✅ **Reviewed:** [PR #32](https://git.eeqj.de/sneak/webhooker/pulls/32) — test coverage improvements for [issue #28](https://git.eeqj.de/sneak/webhooker/issues/28) ### Verification Results | Check | Result | |-------|--------| | Only test files modified | ✅ 5 new files: 3 `_test.go` + 2 `testing.go` helpers, zero production code changes | | No linter/CI/Makefile changes | ✅ No changes to Makefile, Dockerfile, go.mod, go.sum, or config | | Existing test assertions unchanged | ✅ No modifications to `engine_test.go` or `circuit_breaker_test.go` | | Tests exercise real behavior | ✅ See quality assessment below | | No mocking the thing under test | ✅ Test doubles are only for dependencies (httptest servers, in-memory SQLite) | | `docker build .` passes | ✅ Build + `make check` (format, lint, tests) all pass | ### Test Quality Assessment **`internal/delivery/engine_integration_test.go`** (17 tests, 70 assertions) - Real SQLite databases with `AutoMigrate` for proper schema - Real HTTP servers via `httptest` verifying actual request content (Content-Type headers, body content) - Verifies delivery status transitions (pending → delivered, pending → failed) - Tests large body DB fetch path (nil body → fetched from event table) - Tests retry logic including skip-if-not-retrying guard - Tests worker lifecycle (start/stop) with `require.Eventually` for async verification - Tests recovery of pending and retrying deliveries - Tests edge cases: unknown target type → failed, invalid config → failed, timeout → failed **`internal/middleware/middleware_test.go`** (16 tests, 36 assertions) - Tests logging middleware captures status codes and passes through to next handler - Tests CORS in dev mode (allows `*`) vs prod mode (no CORS headers) - Tests `RequireAuth` with real session round-trips (cookie create → set user → re-read) - Tests `MetricsAuth` with valid/invalid/missing basic auth credentials - Tests `ipFromHostPort` helper with table-driven subtests **`internal/session/session_test.go`** (16 tests, 71 assertions) - Full cookie round-trip persistence tests (create session → save → new request with cookie → verify data) - Tests `SetUser`/`GetUserID`/`GetUsername` with actual value verification - Tests `IsAuthenticated` across all states (new, authenticated, cleared, wrong-type value) - Tests `Destroy` sets MaxAge=-1 and clears all user keys - Tests edge cases: overwrite user, destroy-then-save deletes cookie ### Notes - `testing.go` helper files (non-`_test.go`) are compiled into the production binary. This is a standard Go pattern for cross-package test helpers and is harmless since they're in `internal/` packages. No action needed. - All tests use `t.Parallel()` for speed. - Assertion messages are descriptive throughout. Labeling `merge-ready` and assigning to @sneak for final review.

clawbot added

merge-ready

and removed

bot

labels 2026-03-04 01:35:50 +01:00

sneak was assigned by clawbot

2026-03-04 01:36:03 +01:00

sneak merged commit 289f479772 into main

2026-03-04 12:07:24 +01:00

sneak deleted branch fix/issue-28-test-coverage

2026-03-04 12:07:24 +01:00

sneak referenced this issue from a commit