## Summary
Add 32 tests for the `internal/state` package, which previously had 0% test coverage.
### Tests added:
**Save/Load round-trip:**
- Domain, hostname, port, and certificate data all survive save→load cycles
- Error fields (omitempty) round-trip correctly
- Backward-compatible PortState deserialization (old single-hostname → new multi-hostname format)
**Edge cases:**
- Missing state file: returns nil error, keeps existing in-memory state
- Corrupt state file: returns parse error
- Empty state file: returns parse error
- Permission errors (read/write): properly reported, skipped when running as root in Docker
**Atomic write:**
- No leftover .tmp files after successful save
- Updated content verified after second save
**Getter/setter coverage:**
- Domain: get, set, overwrite
- Hostname: get, set with nested nameserver records
- Port: get, set, delete
- Certificate: get, set
- GetAllPortKeys enumeration
- GetSnapshot returns value copy
**Concurrency:**
- 20 goroutines × 50 iterations of concurrent get/set/delete with race detector
- 10 goroutines doing concurrent Save/Load
**Other:**
- Snapshot version written correctly
- LastUpdated timestamp set on save
- File permissions are 0600
- Multiple saves overwrite previous state completely
- NewForTest helper creates valid empty state
- Save creates nested data directories
Also adds `NewForTestWithDataDir()` to the test helper for tests requiring file persistence.
Closes [issue #70](https://git.eeqj.de/sneak/dnswatcher/issues/70)
<!-- session: agent:sdlc-manager:subagent:e75f60a3-17c4-43f7-a743-32a108ee5081 -->
Add 32 tests covering:
- Save/Load round-trip for all state types (domains, hostnames, ports, certificates)
- Atomic write verification (no leftover temp files)
- PortState.UnmarshalJSON backward compatibility (old single-hostname format)
- Missing, corrupt, and empty state file handling
- Permission error handling (skipped when running as root)
- All getter/setter/delete methods for every state type
- GetSnapshot returns a value copy
- GetAllPortKeys enumeration
- Concurrent read/write safety with race detection
- Concurrent Save/Load safety
- File permission verification (0600)
- Multiple saves overwrite previous state
- LastUpdated timestamp updates on save
- Error field round-trips for certificates and hostnames
- Snapshot version correctness
Also adds NewForTestWithDataDir() helper for tests requiring file persistence.
Closes#70
32 new tests for internal/state covering all previously-untested persistence and state management code. Coverage goes from 0% to comprehensive. Clean, well-structured test code.
Checklist
Tests are meaningful: Real assertions on actual data (nameservers, hostnames, ports, certificates, error states). No trivially-passing tests.
No production code changes: Only state_test.go and state_test_helper.go added. Zero production files modified.
No DNS mocking: Tests deal with file persistence and in-memory state 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
Save/Load round-trips: Domains, hostnames, ports, certificates each verified independently
Atomic writes: Verifies no leftover .tmp files after save
Error field round-trips: Both hostname and certificate error states survive save/load
Notes
state_test_helper.go in the state package provides NewForTest() and NewForTestWithDataDir() — correct Go pattern for white-box test helpers accessible from the state_test external package.
All tests use t.Parallel() where safe. The concurrency tests properly use sync.WaitGroup.
Good use of t.TempDir() for isolation and automatic cleanup.
Ready to merge.
## ✅ Review: PASS
### Summary
32 new tests for `internal/state` covering all previously-untested persistence and state management code. Coverage goes from 0% to comprehensive. Clean, well-structured test code.
### Checklist
- [x] **Tests are meaningful**: Real assertions on actual data (nameservers, hostnames, ports, certificates, error states). No trivially-passing tests.
- [x] **No production code changes**: Only `state_test.go` and `state_test_helper.go` added. Zero production files modified.
- [x] **No DNS mocking**: Tests deal with file persistence and in-memory state 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
- **Save/Load round-trips**: Domains, hostnames, ports, certificates each verified independently
- **Atomic writes**: Verifies no leftover `.tmp` files after save
- **Error handling**: Corrupt files, empty files, permission errors (read and write)
- **Backward-compatible deserialization**: Old single-`hostname` → new `hostnames[]` format migration
- **Concurrency safety**: 20 goroutines × 50 iterations of concurrent get/set/delete, plus concurrent save/load
- **Snapshot isolation**: Verifies `GetSnapshot()` returns a copy that doesn't mutate internal state
- **Edge cases**: Missing file (no-op), directory creation on save, file permissions (0600), multiple saves overwriting
- **Error field round-trips**: Both hostname and certificate error states survive save/load
### Notes
- `state_test_helper.go` in the `state` package provides `NewForTest()` and `NewForTestWithDataDir()` — correct Go pattern for white-box test helpers accessible from the `state_test` external package.
- All tests use `t.Parallel()` where safe. The concurrency tests properly use `sync.WaitGroup`.
- Good use of `t.TempDir()` for isolation and automatic cleanup.
Ready to merge.
<!-- session: agent:sdlc-manager:subagent:bb3bec9c-91ec-42df-a01e-a9d78f67aec2 -->
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.
Summary
Add 32 tests for the
internal/statepackage, which previously had 0% test coverage.Tests added:
Save/Load round-trip:
Edge cases:
Atomic write:
Getter/setter coverage:
Concurrency:
Other:
Also adds
NewForTestWithDataDir()to the test helper for tests requiring file persistence.Closes issue #70
✅ Review: PASSED
Summary
Adds 32 comprehensive tests for
internal/state, taking it from 0% coverage to thorough coverage of all public API surface. Closes #70.What was reviewed
state_test.go(+1302 lines),state_test_helper.go(+16 lines — addsNewForTestWithDataDir())internal/state/were modified — no existing tests weakenedTest coverage analysis
All items from issue #70 are addressed:
TestSaveAtomicWrite— verifies no .tmp leftover, second save updates contentTestLoadMissingFile,TestLoadCorruptFile,TestLoadEmptyFileTestConcurrentGetSet(20 goroutines × 50 iterations),TestConcurrentSaveLoad(10 goroutines)TestLoadReadPermissionError,TestSaveWritePermissionError(correctly skip when root in Docker)Code quality
t.Parallel()✅t.TempDir()for file persistence tests (auto-cleanup) ✅t.Helper()✅NewForTestWithDataDir()is minimal and follows the same pattern as existingNewForTest()✅package state_test(black-box testing) ✅Docker build
docker build .passes — all 32 new state tests pass. Permission tests correctly skipped when running as root in Docker container.Verdict: merge-ready ✅
✅ Review: PASS
Summary
32 new tests for
internal/statecovering all previously-untested persistence and state management code. Coverage goes from 0% to comprehensive. Clean, well-structured test code.Checklist
state_test.goandstate_test_helper.goadded. Zero production files modified..golangci.ymlunchanged.docker build .succeeds (all checks pass includingmake check).Test Coverage Highlights
.tmpfiles after savehostname→ newhostnames[]format migrationGetSnapshot()returns a copy that doesn't mutate internal stateNotes
state_test_helper.goin thestatepackage providesNewForTest()andNewForTestWithDataDir()— correct Go pattern for white-box test helpers accessible from thestate_testexternal package.t.Parallel()where safe. The concurrency tests properly usesync.WaitGroup.t.TempDir()for isolation and automatic cleanup.Ready to merge.