Add tests for internal/storage — the backend abstraction has zero coverage #66

New Issue

clawbot · 2026-08-09T03:41:40+02:00

clawbot commented

2026-08-09 03:41:40 +02:00

internal/storage is the pluggable storage backend the README headlines
at :51, and it has no test files at all.

Source files with zero coverage: storer.go, s3.go, file.go,
rclone.go, url.go, module.go.

url.go (142 lines) parses s3://, file://, and rclone:// URLs
including query parameters, and decides which backend gets constructed.
A parsing bug there silently sends backups to the wrong destination. It
has no unit test.

README:466-468 already flags "Storage backend coverage tests… the rclone
path is the least exercised in CI". In fact none of the three backends is
exercised at this layer.

Definition of done

internal/storage has table-driven tests for URL parsing covering, at
minimum: each of the three schemes; query-parameter handling; missing
or malformed components; an unknown scheme; and the exact backend type
selected for each valid input. Assert on error cases, not just happy
paths.
file:// backend: round-trip tests (put, get, list, delete, stat)
against a temp directory via t.TempDir(), including missing-key and
overwrite behavior.
s3:// backend: tests against the existing S3 test harness used by
internal/s3 (which has 445 test LOC — reuse it rather than inventing
a new mock).
rclone:// backend: at minimum, construction and argument-shaping
tests that do not require a live rclone binary. If genuinely
untestable without the binary, say so explicitly in a comment and
cover what can be covered.
The Storer interface contract is exercised through a shared
conformance test that every backend runs, so a new backend inherits
coverage.
No production behavior changes. This issue adds tests only. If a test
uncovers a real bug, file it separately and reference it here rather
than fixing it inline.
make check green.

`internal/storage` is the pluggable storage backend the README headlines at :51, and it has **no test files at all**. Source files with zero coverage: `storer.go`, `s3.go`, `file.go`, `rclone.go`, `url.go`, `module.go`. `url.go` (142 lines) parses `s3://`, `file://`, and `rclone://` URLs including query parameters, and decides which backend gets constructed. A parsing bug there silently sends backups to the wrong destination. It has no unit test. README:466-468 already flags "Storage backend coverage tests… the rclone path is the least exercised in CI". In fact none of the three backends is exercised at this layer. ## Definition of done 1. `internal/storage` has table-driven tests for URL parsing covering, at minimum: each of the three schemes; query-parameter handling; missing or malformed components; an unknown scheme; and the exact backend type selected for each valid input. Assert on error cases, not just happy paths. 2. `file://` backend: round-trip tests (put, get, list, delete, stat) against a temp directory via `t.TempDir()`, including missing-key and overwrite behavior. 3. `s3://` backend: tests against the existing S3 test harness used by `internal/s3` (which has 445 test LOC — reuse it rather than inventing a new mock). 4. `rclone://` backend: at minimum, construction and argument-shaping tests that do not require a live rclone binary. If genuinely untestable without the binary, say so explicitly in a comment and cover what can be covered. 5. The `Storer` interface contract is exercised through a shared conformance test that every backend runs, so a new backend inherits coverage. 6. No production behavior changes. This issue adds tests only. If a test uncovers a real bug, file it separately and reference it here rather than fixing it inline. 7. `make check` green.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:41:40 +02:00

clawbot referenced this issue

2026-08-09 03:44:52 +02:00

Triage the 15 stale remote branches; two may contain unlanded fixes #71

clawbot referenced this issue

2026-08-09 10:13:00 +02:00

Decide whether feature/daemon-mode is 1.0 scope #94

Sign in to join this conversation.