Add real end-to-end tests for the freshen and fetch commands #66

Open
opened 2026-08-09 03:39:26 +02:00 by clawbot · 0 comments
Collaborator

Context

freshen has no test at all. internal/cli/freshen_test.go contains
TestFreshenUnchanged and TestFreshenWithChanges, but neither ever calls
freshenManifestOperation or freshenCommand. The file says so out loud at
internal/cli/freshen_test.go:66-67:

// Note: The freshen operation would need to be run here
// For now, we just verify the test setup is correct.

grep for freshenManifestOperation across all _test.go files returns
nothing. These tests assert that the fixture was built correctly and then
stop. They pass whether or not freshen works, which makes them worse than
no test — they show up as green coverage for a command that is entirely
unverified.

fetch is only half tested. internal/cli/fetch_test.go has four tests
(TestFetchFromHTTP, TestFetchHashMismatch, TestFetchSizeMismatch,
TestFetchProgress) which all call downloadFile() directly against an
httptest.Server. That covers the download primitive well, but nothing
calls fetchManifestOperation or fetchCommand, so manifest resolution,
iteration over entries, directory creation, resume behavior, and the
command's exit codes are untested.

Definition of done

  • freshen is tested end-to-end through freshenManifestOperation (or the
    command entry point): build a tree, generate a manifest, then verify each
    of — no changes leaves the manifest semantically unchanged; a modified
    file updates its hash and size; a new file is added; a deleted file is
    removed; the manifest on disk is rewritten correctly and reloads.
  • The placeholder comment at internal/cli/freshen_test.go:66-67 is gone,
    along with any assertion that only checks fixture setup.
  • fetch is tested end-to-end through fetchManifestOperation (or the
    command entry point) against an httptest.Server serving a real
    generated manifest plus its files: all files land with correct content,
    nested directories are created, a hash mismatch fails the command with a
    non-zero exit, and a partially-present destination resumes or re-fetches
    per the documented behavior.
  • Both suites run within the make test timeout and use t.TempDir().
  • make check passes. TODO.md updated in the same commit.

Implementation requirements

  • Do not delete the existing tests to make room; replace their bodies with
    real assertions, or add alongside and remove the placeholders explicitly.
  • No network access. httptest.Server only.
  • No gpg dependency in these tests — signing is covered separately, and
    these must not skip when gpg is absent.
  • Assert on observable outcomes: file contents on disk, the reloaded
    manifest, exit codes. Do not assert on log text as a proxy for behavior.
  • If writing these tests reveals actual bugs in freshen or fetch
    which is likely, given neither has ever been exercised — do not paper over
    them to get green. File each as its own issue, reference it from the PR,
    and mark the failing case t.Skip with the issue number only if fixing it
    in the same PR would balloon the scope. Fixing it in place is preferred.
  • Commit title must end with (closes #66).
## Context **`freshen` has no test at all.** `internal/cli/freshen_test.go` contains `TestFreshenUnchanged` and `TestFreshenWithChanges`, but neither ever calls `freshenManifestOperation` or `freshenCommand`. The file says so out loud at `internal/cli/freshen_test.go:66-67`: ``` // Note: The freshen operation would need to be run here // For now, we just verify the test setup is correct. ``` `grep` for `freshenManifestOperation` across all `_test.go` files returns nothing. These tests assert that the fixture was built correctly and then stop. They pass whether or not `freshen` works, which makes them worse than no test — they show up as green coverage for a command that is entirely unverified. **`fetch` is only half tested.** `internal/cli/fetch_test.go` has four tests (`TestFetchFromHTTP`, `TestFetchHashMismatch`, `TestFetchSizeMismatch`, `TestFetchProgress`) which all call `downloadFile()` directly against an `httptest.Server`. That covers the download primitive well, but nothing calls `fetchManifestOperation` or `fetchCommand`, so manifest resolution, iteration over entries, directory creation, resume behavior, and the command's exit codes are untested. ## Definition of done - `freshen` is tested end-to-end through `freshenManifestOperation` (or the command entry point): build a tree, generate a manifest, then verify each of — no changes leaves the manifest semantically unchanged; a modified file updates its hash and size; a new file is added; a deleted file is removed; the manifest on disk is rewritten correctly and reloads. - The placeholder comment at `internal/cli/freshen_test.go:66-67` is gone, along with any assertion that only checks fixture setup. - `fetch` is tested end-to-end through `fetchManifestOperation` (or the command entry point) against an `httptest.Server` serving a real generated manifest plus its files: all files land with correct content, nested directories are created, a hash mismatch fails the command with a non-zero exit, and a partially-present destination resumes or re-fetches per the documented behavior. - Both suites run within the `make test` timeout and use `t.TempDir()`. - `make check` passes. `TODO.md` updated in the same commit. ## Implementation requirements - Do not delete the existing tests to make room; replace their bodies with real assertions, or add alongside and remove the placeholders explicitly. - No network access. `httptest.Server` only. - No gpg dependency in these tests — signing is covered separately, and these must not skip when gpg is absent. - Assert on observable outcomes: file contents on disk, the reloaded manifest, exit codes. Do not assert on log text as a proxy for behavior. - If writing these tests reveals actual bugs in `freshen` or `fetch` — which is likely, given neither has ever been exercised — do not paper over them to get green. File each as its own issue, reference it from the PR, and mark the failing case `t.Skip` with the issue number only if fixing it in the same PR would balloon the scope. Fixing it in place is preferred. - Commit title must end with ` (closes #66)`.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:39:26 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/mfer#66