sneak/dnswatcher
1
0
Fork 0
Code Issues 3 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

test(config): add comprehensive tests for config loading path #81

Merged
sneak merged 1 commits from fix/72-config-test-coverage into main 2026-03-04 11:23:25 +01:00
Conversation 2 Commits 1 Files Changed 3 +310
clawbot commented 2026-03-02 08:57:07 +01:00
Collaborator
Copy Link

Summary

Add comprehensive tests for the internal/config package, covering the main configuration loading path that was previously untested.

Closes issue #72

What Changed

Added three new test files:

  • config_test.go — 16 tests covering New(), StatePath(), and the full config loading pipeline
  • parsecsv_test.go — 10 test cases for parseCSV() edge cases
  • export_test.go — standard Go export bridge for testing unexported parseCSV

Test Coverage

Area Tests
Default values All 14 config fields verified against documented defaults
Environment overrides All env vars tested including PORT (unprefixed)
Invalid duration fallback DNSWATCHER_DNS_INTERVAL=banana falls back to 1h
Invalid TLS interval DNSWATCHER_TLS_INTERVAL=notaduration falls back to 12h
No targets error Empty/missing DNSWATCHER_TARGETS returns ErrNoTargets
Invalid targets Public suffix (co.uk) rejected with error
CSV parsing Trailing commas, leading commas, consecutive commas, whitespace, tabs
Debug mode DNSWATCHER_DEBUG=true enables debug logging
Target classification Domains vs hostnames correctly separated via PSL
StatePath Path construction with various DataDir values
Empty appname Falls back to "dnswatcher" config file name

Coverage: 23% → 92.5%

Notes

  • Tests use viper.Reset() for isolation since Viper has global state
  • Non-parallel tests use t.Setenv() for automatic env var cleanup
  • Uses testify assert/require consistent with other test files in the repo
  • No production code changes
## Summary Add comprehensive tests for the `internal/config` package, covering the main configuration loading path that was previously untested. Closes [issue #72](https://git.eeqj.de/sneak/dnswatcher/issues/72) ## What Changed Added three new test files: - **`config_test.go`** — 16 tests covering `New()`, `StatePath()`, and the full config loading pipeline - **`parsecsv_test.go`** — 10 test cases for `parseCSV()` edge cases - **`export_test.go`** — standard Go export bridge for testing unexported `parseCSV` ## Test Coverage | Area | Tests | |------|-------| | Default values | All 14 config fields verified against documented defaults | | Environment overrides | All env vars tested including `PORT` (unprefixed) | | Invalid duration fallback | `DNSWATCHER_DNS_INTERVAL=banana` falls back to 1h | | Invalid TLS interval | `DNSWATCHER_TLS_INTERVAL=notaduration` falls back to 12h | | No targets error | Empty/missing `DNSWATCHER_TARGETS` returns `ErrNoTargets` | | Invalid targets | Public suffix (`co.uk`) rejected with error | | CSV parsing | Trailing commas, leading commas, consecutive commas, whitespace, tabs | | Debug mode | `DNSWATCHER_DEBUG=true` enables debug logging | | Target classification | Domains vs hostnames correctly separated via PSL | | StatePath | Path construction with various `DataDir` values | | Empty appname | Falls back to "dnswatcher" config file name | **Coverage: 23% → 92.5%** ## Notes - Tests use `viper.Reset()` for isolation since Viper has global state - Non-parallel tests use `t.Setenv()` for automatic env var cleanup - Uses testify `assert`/`require` consistent with other test files in the repo - No production code changes <!-- session: agent:sdlc-manager:subagent:d7fe6cf2-4746-4793-a738-9df8f5f5f0c6 -->
clawbot added 1 commit 2026-03-02 08:57:07 +01:00
test(config): add comprehensive tests for config loading path
All checks were successful
check / check (push) Successful in 43s
Details
0219d7e6e1 
Add tests covering the main configuration loading path which was
previously untested (only ClassifyTargets and PSL logic had tests).

New test coverage:
- New() constructor with default values, env overrides, and error cases
- buildConfig() interval parsing with fallback to defaults on invalid input
- setupViper() env prefix binding and default value registration
- parseCSV() edge cases: whitespace, trailing/leading commas, empty segments
- configureDebugLogging() debug mode activation
- StatePath() path construction with various DataDir values
- ErrNoTargets sentinel error for missing/empty targets
- PORT env without DNSWATCHER_ prefix (compatibility)
- Target classification into domains vs hostnames via PSL
- Invalid targets (public suffixes) rejected with clear error

Coverage for internal/config increased from 23% to 92.5%.
clawbot added the botneeds-review labels 2026-03-02 08:57:24 +01:00
clawbot commented 2026-03-02 09:03:11 +01:00
Author
Collaborator
Copy Link

✅ Review: PASS

Summary

25+ new tests for internal/config covering config loading, environment overrides, edge cases, and the parseCSV helper. Coverage goes from 23% to ~92.5%. Clean, well-structured test code.

Checklist

  • Tests are meaningful: Real assertions on actual config field values, error conditions, and edge cases. No trivially-passing tests.
  • No production code changes: Only config_test.go, export_test.go, and parsecsv_test.go added. Zero production files modified.
  • No DNS mocking: Tests deal with config parsing and environment variables only. No DNS client interaction.
  • No weakened existing tests: No pre-existing test files modified.
  • No CI/linter config changes: Dockerfile, Makefile, .golangci.yml unchanged.
  • CI passes: docker build . succeeds (all checks pass including make check).
  • Branch is up to date with main: No rebase needed.

Test Coverage Highlights

  • Default values: Verifies all 13+ config fields have correct defaults when only DNSWATCHER_TARGETS is set
  • Environment overrides: Every config field verified with custom env var values (Port, Debug, DataDir, DNSInterval, TLSInterval, TLSExpiryWarning, SlackWebhook, MattermostWebhook, NtfyTopic, SentryDSN, MaintenanceMode, MetricsUsername, MetricsPassword)
  • Error handling: Missing targets (ErrNoTargets), empty CSV segments, public suffix rejection
  • Duration parsing edge cases: Invalid DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL fall back to defaults
  • Target classification: Apex domains vs hostnames correctly separated
  • parseCSV edge cases: Empty string, whitespace, trailing/leading commas, consecutive commas, tabs — 10 test cases
  • StatePath: Multiple data directory variants verified
  • PORT env: Non-prefixed PORT env var works (not DNSWATCHER_PORT)

Notes

  • Config tests correctly avoid t.Parallel() due to viper global state — documented with a comment. Each test resets viper for isolation.
  • export_test.go exports parseCSV for external package testing — correct Go pattern.
  • Good use of t.Setenv() for env var isolation (automatically restored after each test).
  • Uses testify/assert and testify/require consistently, matching the project's existing test style.

Ready to merge.

## ✅ Review: PASS ### Summary 25+ new tests for `internal/config` covering config loading, environment overrides, edge cases, and the `parseCSV` helper. Coverage goes from 23% to ~92.5%. Clean, well-structured test code. ### Checklist - [x] **Tests are meaningful**: Real assertions on actual config field values, error conditions, and edge cases. No trivially-passing tests. - [x] **No production code changes**: Only `config_test.go`, `export_test.go`, and `parsecsv_test.go` added. Zero production files modified. - [x] **No DNS mocking**: Tests deal with config parsing and environment variables only. No DNS client interaction. - [x] **No weakened existing tests**: No pre-existing test files modified. - [x] **No CI/linter config changes**: Dockerfile, Makefile, `.golangci.yml` unchanged. - [x] **CI passes**: `docker build .` succeeds (all checks pass including `make check`). - [x] **Branch is up to date with main**: No rebase needed. ### Test Coverage Highlights - **Default values**: Verifies all 13+ config fields have correct defaults when only `DNSWATCHER_TARGETS` is set - **Environment overrides**: Every config field verified with custom env var values (Port, Debug, DataDir, DNSInterval, TLSInterval, TLSExpiryWarning, SlackWebhook, MattermostWebhook, NtfyTopic, SentryDSN, MaintenanceMode, MetricsUsername, MetricsPassword) - **Error handling**: Missing targets (`ErrNoTargets`), empty CSV segments, public suffix rejection - **Duration parsing edge cases**: Invalid `DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL` fall back to defaults - **Target classification**: Apex domains vs hostnames correctly separated - **`parseCSV` edge cases**: Empty string, whitespace, trailing/leading commas, consecutive commas, tabs — 10 test cases - **`StatePath`**: Multiple data directory variants verified - **PORT env**: Non-prefixed `PORT` env var works (not `DNSWATCHER_PORT`) ### Notes - Config tests correctly avoid `t.Parallel()` due to viper global state — documented with a comment. Each test resets viper for isolation. - `export_test.go` exports `parseCSV` for external package testing — correct Go pattern. - Good use of `t.Setenv()` for env var isolation (automatically restored after each test). - Uses `testify/assert` and `testify/require` consistently, matching the project's existing test style. Ready to merge. <!-- session: agent:sdlc-manager:subagent:bb3bec9c-91ec-42df-a01e-a9d78f67aec2 -->
clawbot added merge-ready and removed needs-reviewbot labels 2026-03-02 09:03:36 +01:00
sneak was assigned by clawbot 2026-03-02 09:03:53 +01:00
clawbot commented 2026-03-02 09:04:30 +01:00
Author
Collaborator
Copy Link

✅ Review: PASS

Summary

Excellent test coverage for the internal/config package. Takes coverage from 23% to 92.5% with clean, well-structured tests that exercise the full config loading pipeline.

What Was Reviewed

  • config_test.go (260 lines) — 16 tests covering New(), StatePath(), and the full config pipeline
  • parsecsv_test.go (44 lines) — 10 test cases for parseCSV() edge cases
  • export_test.go (6 lines) — standard Go export bridge for parseCSV

Checklist

  • Defaults verified: All 14 config fields checked against documented defaults
  • Env overrides: All env vars tested including unprefixed PORT
  • Invalid duration fallback: banana → 1h, notaduration → 12h, both invalid simultaneously
  • No targets error: Empty string and whitespace-only CSV both return ErrNoTargets
  • Invalid targets: Public suffix (co.uk) correctly rejected
  • CSV edge cases: Trailing/leading/consecutive commas, whitespace, tabs, all-empty segments
  • Debug mode: Enables debug logging
  • Target classification: Domains vs hostnames correctly separated via PSL
  • StatePath: 4 sub-cases (default, absolute, nested, empty)
  • Empty appname: Falls back to dnswatcher config file name
  • No DNS mocking — config tests don't involve DNS (as expected)
  • No production code changes — 310 additions, 0 deletions, test files only
  • No weakening of existing tests — classify_test.go untouched
  • docker build . passes (no-cache, all tests green, lint clean)
  • Viper isolation: viper.Reset() before each test, t.Setenv() for env cleanup
  • Consistent style: Uses testify/assert + testify/require, matches existing test patterns
  • Parallel where safe: TestStatePath and TestParseCSV use t.Parallel(), Viper-dependent tests correctly sequential

Notes

  • Clean design decision to use export_test.go bridge pattern rather than making parseCSV public
  • Good coverage of the Viper global state footgun — each test properly resets state
  • Test names are descriptive and follow Go conventions

No issues found. Ready to merge.

## ✅ Review: PASS ### Summary Excellent test coverage for the `internal/config` package. Takes coverage from 23% to 92.5% with clean, well-structured tests that exercise the full config loading pipeline. ### What Was Reviewed - **`config_test.go`** (260 lines) — 16 tests covering `New()`, `StatePath()`, and the full config pipeline - **`parsecsv_test.go`** (44 lines) — 10 test cases for `parseCSV()` edge cases - **`export_test.go`** (6 lines) — standard Go export bridge for `parseCSV` ### Checklist - [x] **Defaults verified**: All 14 config fields checked against documented defaults - [x] **Env overrides**: All env vars tested including unprefixed `PORT` - [x] **Invalid duration fallback**: `banana` → 1h, `notaduration` → 12h, both invalid simultaneously - [x] **No targets error**: Empty string and whitespace-only CSV both return `ErrNoTargets` - [x] **Invalid targets**: Public suffix (`co.uk`) correctly rejected - [x] **CSV edge cases**: Trailing/leading/consecutive commas, whitespace, tabs, all-empty segments - [x] **Debug mode**: Enables debug logging - [x] **Target classification**: Domains vs hostnames correctly separated via PSL - [x] **StatePath**: 4 sub-cases (default, absolute, nested, empty) - [x] **Empty appname**: Falls back to `dnswatcher` config file name - [x] **No DNS mocking** — config tests don't involve DNS (as expected) - [x] **No production code changes** — 310 additions, 0 deletions, test files only - [x] **No weakening of existing tests** — `classify_test.go` untouched - [x] **`docker build .` passes** (no-cache, all tests green, lint clean) - [x] **Viper isolation**: `viper.Reset()` before each test, `t.Setenv()` for env cleanup - [x] **Consistent style**: Uses `testify/assert` + `testify/require`, matches existing test patterns - [x] **Parallel where safe**: `TestStatePath` and `TestParseCSV` use `t.Parallel()`, Viper-dependent tests correctly sequential ### Notes - Clean design decision to use `export_test.go` bridge pattern rather than making `parseCSV` public - Good coverage of the Viper global state footgun — each test properly resets state - Test names are descriptive and follow Go conventions No issues found. Ready to merge.
sneak merged commit d6130e5892 into main 2026-03-04 11:23:25 +01:00
sneak deleted branch fix/72-config-test-coverage 2026-03-04 11:23:25 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bot
Agent's turn to work
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#81
Delete Branch "fix/72-config-test-coverage"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?