State package has 0% test coverage #70

New Issue

clawbot · 2026-03-02T00:51:01+01:00

clawbot commented

2026-03-02 00:51:01 +01:00

Problem

The internal/state package has 0% test coverage. It only contains a state_test_helper.go file (which provides NewForTest() for other packages) but no actual tests for the state package itself.

What is Untested

Load() — reading and parsing state from disk
Save() — atomic write (temp file + rename)
GetSnapshot() — snapshot copying
All getter/setter methods (Get/Set/Delete for domains, hostnames, ports, certificates)
PortState.UnmarshalJSON() — backward-compatible deserialization (old single-hostname format → new multi-hostname format)
Edge cases: corrupt state file, partial writes, permission errors, empty state file, version mismatch

Why This Matters

The state file is the daemon's persistence layer. Bugs here cause:

Data loss (failed saves)
False-positive notifications on restart (failed loads)
Corruption from non-atomic writes
Silent data migration failures (backward compat)

Recommendation

Add tests covering at minimum:

Save → Load round-trip preserves all data
Atomic write: interrupted save doesn't corrupt existing state
Backward-compatible PortState deserialization
Missing/corrupt state file handling
Concurrent access (the package uses sync.RWMutex)

Category

Should-fix before 1.0. State persistence is a core feature and should have test coverage.

## Problem The `internal/state` package has **0% test coverage**. It only contains a `state_test_helper.go` file (which provides `NewForTest()` for other packages) but no actual tests for the state package itself. ## What is Untested - `Load()` — reading and parsing state from disk - `Save()` — atomic write (temp file + rename) - `GetSnapshot()` — snapshot copying - All getter/setter methods (Get/Set/Delete for domains, hostnames, ports, certificates) - `PortState.UnmarshalJSON()` — backward-compatible deserialization (old single-hostname format → new multi-hostname format) - Edge cases: corrupt state file, partial writes, permission errors, empty state file, version mismatch ## Why This Matters The state file is the daemon's persistence layer. Bugs here cause: - Data loss (failed saves) - False-positive notifications on restart (failed loads) - Corruption from non-atomic writes - Silent data migration failures (backward compat) ## Recommendation Add tests covering at minimum: 1. Save → Load round-trip preserves all data 2. Atomic write: interrupted save doesn't corrupt existing state 3. Backward-compatible PortState deserialization 4. Missing/corrupt state file handling 5. Concurrent access (the package uses sync.RWMutex) ## Category Should-fix before 1.0. State persistence is a core feature and should have test coverage.

clawbot added the bot label 2026-03-02 00:51:01 +01:00

clawbot referenced this issue

2026-03-02 00:55:17 +01:00

1.0/mvp #66

clawbot referenced this issue

2026-03-02 00:58:13 +01:00

1.0/mvp #66

clawbot referenced this issue

2026-03-02 08:41:55 +01:00

1.0/mvp #66

clawbot referenced this issue

2026-03-02 08:46:27 +01:00