From 6476fa831a787237fa6701c0e60c8e4ce819031d Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 13 Oct 2026 19:43:25 +0000 Subject: [PATCH] Silence prune and snapshot removal status under --cron and --quiet (closes #294) PruneBlobs, CleanupLocalSnapshots, RemoveSnapshot, RemoveAllSnapshots and snapshot purge wrote their status lines straight to stdout, so `snapshot create --cron --prune` printed on every successful run and --quiet did not silence `prune`, `snapshot purge`, `snapshot remove` or `remote nuke`. Those lines now go through internal/ui, which drops them under --cron and --quiet; warnings, errors and --json documents still print. The confirmation prompts still write straight to stdout, as the `database delete` prompt does, together with what they ask to delete: the snapshot list before the purge prompt, and the blob count and size before the prune prompt. With --force those lines are status lines. The two purge test fixtures gained the UI writer production always sets. Model: opus-5-5 --- README.md | 6 +- internal/cli/entry_prune_quiet_test.go | 123 +++++++++++++++ internal/vaultik/prune.go | 13 +- .../purge_local_remote_consistency_test.go | 6 +- internal/vaultik/purge_per_name_test.go | 2 + internal/vaultik/remove_quiet_test.go | 148 ++++++++++++++++++ internal/vaultik/remove_snapshot_test.go | 4 +- internal/vaultik/snapshot.go | 76 +++++---- 8 files changed, 337 insertions(+), 41 deletions(-) create mode 100644 internal/cli/entry_prune_quiet_test.go create mode 100644 internal/vaultik/remove_quiet_test.go diff --git a/README.md b/README.md index ee375b0..3ba0768 100644 --- a/README.md +++ b/README.md @@ -712,8 +712,10 @@ the `NO_COLOR` environment variable is unset (https://no-color.org/). reports and tables (`version`, `info`, `remote info`, `snapshot verify`, `snapshot list`) but never the machine-consumed `config get` value or the `--json` documents, which a script depends on. The - `database delete` confirmation prompt is also written this way and - always shown: it is an interactive exchange the operator must see. + confirmation prompts of `database delete`, `prune`, `snapshot purge` + and `snapshot remove`, each with what it asks to delete, are also + written this way and always shown: they are an interactive exchange + the operator must see. `internal/ui` writes to stdout; it is the output the user asked for. The exceptions are the startup banner and the error a failed command ends diff --git a/internal/cli/entry_prune_quiet_test.go b/internal/cli/entry_prune_quiet_test.go new file mode 100644 index 0000000..92e7a3b --- /dev/null +++ b/internal/cli/entry_prune_quiet_test.go @@ -0,0 +1,123 @@ +package cli //nolint:testpackage // shares runEntry and the hermetic config helpers + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// flagCron is used to build the argument vectors below. +const flagCron = "--cron" + +// plantUnreferencedBlob writes a blob that no snapshot references to the +// destination store at storeDir, so that a prune deletes it. Returns its +// path. +func plantUnreferencedBlob(t *testing.T, storeDir string) string { + t.Helper() + + hash := strings.Repeat("b", 64) + path := filepath.Join(storeDir, "blobs", hash[:2], hash[2:4], hash) + + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o750)) + require.NoError(t, os.WriteFile(path, []byte("unreferenced"), 0o600)) + + return path +} + +// TestEntrySnapshotCreatePruneIsSilent checks that a successful +// `snapshot create --prune` under --cron or --quiet prints nothing on +// stdout or stderr, whether or not the prune finds a blob to delete. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntrySnapshotCreatePruneIsSilent(t *testing.T) { + for _, testCase := range []struct { + name string + args []string + plantBlob bool + }{ + { + name: "cron, no blob to delete", + args: []string{cmdSnapshot, cmdCreate, flagCron, flagPrune}, + }, + { + name: "cron, blob to delete", + args: []string{cmdSnapshot, cmdCreate, flagCron, flagPrune}, + plantBlob: true, + }, + { + name: "cron with keep-newer-than", + args: []string{ + cmdSnapshot, cmdCreate, flagCron, flagPrune, + flagKeepNewerThan, "4w", + }, + plantBlob: true, + }, + { + name: "quiet", + args: []string{flagQuiet, cmdSnapshot, cmdCreate, flagPrune}, + plantBlob: true, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + configPath, storeDir := writeBackupConfig(t) + + var blob string + if testCase.plantBlob { + blob = plantUnreferencedBlob(t, storeDir) + } + + code, stdout, stderr := runEntry(t, + append([]string{flagConfig, configPath}, testCase.args...)...) + + assert.Equal(t, 0, code) + assert.Empty(t, stdout) + assert.Empty(t, stderr) + + if testCase.plantBlob { + assert.NoFileExists(t, blob, "the prune must delete the blob") + } + }) + } +} + +// TestEntryPruneQuietIsSilent checks that a successful `prune --force` +// under --quiet prints nothing on stdout or stderr, both when there is +// nothing to remove and when it removes a stale local snapshot record +// and an unreferenced blob. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntryPruneQuietIsSilent(t *testing.T) { + for _, testCase := range []struct { + name string + staleRecordAndBlob bool + }{ + {name: "nothing to remove"}, + {name: "stale record and blob to remove", staleRecordAndBlob: true}, + } { + t.Run(testCase.name, func(t *testing.T) { + configPath := writeHermeticPruneConfig(t, testCase.staleRecordAndBlob) + + var blob string + if testCase.staleRecordAndBlob { + // writeHermeticPruneConfig puts the store beside the config. + blob = plantUnreferencedBlob(t, + filepath.Join(filepath.Dir(configPath), "store")) + } + + code, stdout, stderr := runEntry(t, + flagConfig, configPath, flagQuiet, cmdPrune, flagForce) + + assert.Equal(t, 0, code) + assert.Empty(t, stdout) + assert.Empty(t, stderr) + + if testCase.staleRecordAndBlob { + assert.NoFileExists(t, blob, "the prune must delete the blob") + } + }) + } +} diff --git a/internal/vaultik/prune.go b/internal/vaultik/prune.go index d32f68c..6d07e41 100644 --- a/internal/vaultik/prune.go +++ b/internal/vaultik/prune.go @@ -204,7 +204,7 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error { return v.outputPruneBlobsJSON(result) } - v.printlnStdout("No unreferenced blobs to remove.") + v.UI.Infof("No unreferenced blobs to remove.") return nil } @@ -212,12 +212,17 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error { log.Info("Found unreferenced blobs", "count", len(unreferencedBlobs), "total_size", ubytes(totalSize)) - if !opts.JSON { - v.stdoutf("Found %d unreferenced blob(s) totaling %s\n", + if opts.Force && !opts.JSON { + v.UI.Infof("Found %d unreferenced blob(s) totaling %s.", len(unreferencedBlobs), ubytes(totalSize)) } if !opts.Force && !opts.JSON { + // The count, the prompt and its answer bypass the UI: they are an + // interactive exchange the operator must see, so --quiet does not + // silence them. + v.stdoutf("Found %d unreferenced blob(s) totaling %s.\n", + len(unreferencedBlobs), ubytes(totalSize)) v.stdoutf("\nDelete %d unreferenced blob(s)? [y/N] ", len(unreferencedBlobs)) var confirm string @@ -244,7 +249,7 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error { return err } } else { - v.stdoutf("\nDeleted %d blob(s) totaling %s\n", + v.UI.Infof("Deleted %d blob(s) totaling %s.", result.BlobsDeleted, ubytes(result.BytesFreed)) } diff --git a/internal/vaultik/purge_local_remote_consistency_test.go b/internal/vaultik/purge_local_remote_consistency_test.go index f170d2f..c3d827f 100644 --- a/internal/vaultik/purge_local_remote_consistency_test.go +++ b/internal/vaultik/purge_local_remote_consistency_test.go @@ -14,6 +14,7 @@ import ( "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/types" + "sneak.berlin/go/vaultik/internal/ui" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -58,13 +59,16 @@ func setupConsistencyTest( require.NoError(t, err) } + stdout := &bytes.Buffer{} + v := &vaultik.Vaultik{ Storage: mockStorage, Repositories: repos, DB: db, - Stdout: &bytes.Buffer{}, + Stdout: stdout, Stderr: &bytes.Buffer{}, Stdin: &bytes.Buffer{}, + UI: ui.NewWithColor(stdout, false), } v.SetContext(ctx) diff --git a/internal/vaultik/purge_per_name_test.go b/internal/vaultik/purge_per_name_test.go index f895f61..8b537b8 100644 --- a/internal/vaultik/purge_per_name_test.go +++ b/internal/vaultik/purge_per_name_test.go @@ -14,6 +14,7 @@ import ( "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/types" + "sneak.berlin/go/vaultik/internal/ui" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -86,6 +87,7 @@ func setupPurgeTest( Stdout: stdout, Stderr: stderr, Stdin: stdin, + UI: ui.NewWithColor(stdout, false), } v.SetContext(ctx) diff --git a/internal/vaultik/remove_quiet_test.go b/internal/vaultik/remove_quiet_test.go new file mode 100644 index 0000000..c8916fd --- /dev/null +++ b/internal/vaultik/remove_quiet_test.go @@ -0,0 +1,148 @@ +package vaultik_test + +import ( + "bytes" + "context" + "encoding/json" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestPurgeSnapshots_QuietSuppressesReport checks that `snapshot purge +// --keep-latest --force` under --quiet deletes the older snapshot and +// prints nothing. +func TestPurgeSnapshots_QuietSuppressesReport(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + v := setupPurgeTest(t, snapHostname, []string{snapHomeT0, snapHomeT1}) + + var out bytes.Buffer + + v.Stdout = &out + v.UI = ui.NewWithColor(&out, false) + v.UI.SetQuiet(true) + + require.NoError(t, v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ + KeepLatest: true, + Force: true, + })) + + assert.Equal(t, []string{snapHomeT1}, listRemainingSnapshots(t, v)) + assert.Empty(t, out.String()) +} + +// TestPurgeSnapshots_QuietPromptListsSnapshots checks that `snapshot +// purge --keep-latest` without --force still lists the snapshots its +// prompt asks to delete under --quiet. +func TestPurgeSnapshots_QuietPromptListsSnapshots(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + v := setupPurgeTest(t, snapHostname, []string{snapHomeT0, snapHomeT1}) + + var out bytes.Buffer + + v.Stdout = &out + v.UI = ui.NewWithColor(&out, false) + v.UI.SetQuiet(true) + + // Stdin is empty, so the prompt reads no answer and cancels. + require.NoError(t, v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ + KeepLatest: true, + })) + + assert.Contains(t, out.String(), snapHomeT0) + assert.Contains(t, out.String(), "Delete 1 snapshot(s)? [y/N]") + assert.Len(t, listRemainingSnapshots(t, v), 2) +} + +// TestPruneBlobs_QuietPromptShowsBlobTotal checks that `prune` without +// --force still states the count and total size of the blobs its prompt +// asks to delete under --quiet. +func TestPruneBlobs_QuietPromptShowsBlobTotal(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + env := newListEnv(t) + env.v.UI.SetQuiet(true) + + // The store holds no manifest, so nothing references this blob. + addBlob(t, env.store.testStorer, testBlobHashA) + + // Stdin is empty, so the prompt reads no answer and cancels. + require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{})) + + assert.Contains(t, env.stdout.String(), + "Found 1 unreferenced blob(s) totaling") + assert.Contains(t, env.stdout.String(), + "Delete 1 unreferenced blob(s)? [y/N]") +} + +// TestRemoveSnapshot_QuietSuppressesReport checks that `snapshot remove` +// under --quiet prints nothing with --dry-run or with --force, and that +// the --json document still prints. +func TestRemoveSnapshot_QuietSuppressesReport(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + newQuietRemover := func() (*vaultik.TestVaultik, *testStorer) { + store := newTestStorer() + addManifest(t, store, "snapshot-001", nil) + + tv := vaultik.NewForTesting(store) + tv.Repositories = newLocalIndex(t, "snapshot-001") + tv.UI.SetQuiet(true) + + return tv, store + } + + tv, store := newQuietRemover() + + _, err := tv.RemoveSnapshot("snapshot-001", + &vaultik.RemoveOptions{DryRun: true}) + require.NoError(t, err) + assert.Empty(t, tv.Stdout.String(), + "--dry-run must print nothing under --quiet") + + _, err = tv.RemoveSnapshot("snapshot-001", + &vaultik.RemoveOptions{Force: true}) + require.NoError(t, err) + assert.False(t, store.hasKey(remoteKeyPath("snapshot-001"))) + assert.Empty(t, tv.Stdout.String(), + "--force must print nothing under --quiet") + + tv, _ = newQuietRemover() + + _, err = tv.RemoveSnapshot("snapshot-001", + &vaultik.RemoveOptions{JSON: true}) + require.NoError(t, err) + + var result vaultik.RemoveResult + + require.NoError(t, json.Unmarshal(tv.Stdout.Bytes(), &result), + "the --json document must still print under --quiet") + assert.True(t, result.RemoteRemoved) +} + +// TestNukeRemote_QuietSuppressesReport checks that `remote nuke --force` +// under --quiet prints nothing while it removes a snapshot's metadata +// and its blobs. +func TestNukeRemote_QuietSuppressesReport(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + ctx := context.Background() + storeDir := filepath.Join(t.TempDir(), "store") + v, _, out := backUpToFileDestination(ctx, t, storeDir) + v.UI.SetQuiet(true) + + require.NoError(t, v.NukeRemote(true)) + assert.Empty(t, out.String()) +} diff --git a/internal/vaultik/remove_snapshot_test.go b/internal/vaultik/remove_snapshot_test.go index 7d85bea..77dfc2f 100644 --- a/internal/vaultik/remove_snapshot_test.go +++ b/internal/vaultik/remove_snapshot_test.go @@ -350,7 +350,7 @@ func TestRemoveSnapshot_DryRun(t *testing.T) { assert.True(t, store.hasKey("blobs/aa/aa/"+blobA)) assert.True(t, store.hasKey(remoteKeyPath("snapshot-001"))) - assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]") + assert.Contains(t, tv.Stdout.String(), "Dry run: no changes made.") } // TestRemoveSnapshot_UnknownIDIsError checks that an ID naming no @@ -504,7 +504,7 @@ func TestRemoveAllSnapshots_DryRun(t *testing.T) { assert.Equal(t, initialCount, store.keyCount()) - assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]") + assert.Contains(t, tv.Stdout.String(), "Dry run: no changes made.") } func TestRemoveAllSnapshots_NoSnapshots(t *testing.T) { diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index cef9cbf..63a9809 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -574,7 +574,7 @@ func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error { if len(toDelete) == 0 { if !opts.Quiet { - v.printlnStdout("No snapshots to delete") + v.UI.Infof("No snapshots to delete.") } return nil @@ -631,19 +631,19 @@ func selectSnapshotsToPurge( func (v *Vaultik) confirmAndExecutePurge( toDelete []SnapshotInfo, force, quiet bool, ) error { - if !quiet { + // Confirm unless --force is used. The list, the prompt and its answer + // bypass the UI: they are an interactive exchange the operator must + // see, so --quiet does not silence them. + if !force { v.stdoutf("The following snapshots will be deleted:\n\n") for _, snap := range toDelete { v.stdoutf(" %s (%s, %s)\n", snap.ID, - snap.Timestamp.Format("2006-01-02 15:04:05"), + snap.Timestamp.Format(time.DateTime), ubytes(snap.CompressedSize)) } - } - // Confirm unless --force is used - if !force { v.stdoutf("\nDelete %d snapshot(s)? [y/N] ", len(toDelete)) var confirm string @@ -661,7 +661,16 @@ func (v *Vaultik) confirmAndExecutePurge( return nil } } else if !quiet { - v.stdoutf("\nDeleting %d snapshot(s) (--force specified)\n", len(toDelete)) + v.UI.Infof("The following snapshots will be deleted:") + + for _, snap := range toDelete { + v.UI.Detailf("%s (%s, %s)", + v.UI.Snapshot(snap.ID.String()), + snap.Timestamp.Format(time.DateTime), + ubytes(snap.CompressedSize)) + } + + v.UI.Infof("Deleting %d snapshot(s) (--force specified).", len(toDelete)) } // Delete snapshots (both local and remote) @@ -694,9 +703,9 @@ func (v *Vaultik) confirmAndExecutePurge( } if !quiet { - v.stdoutf("Deleted %d snapshot(s)\n", len(toDelete)) - v.printlnStdout( - "\nNote: Run 'vaultik prune' to clean up unreferenced remote blobs.") + v.UI.Infof("Deleted %d snapshot(s).", len(toDelete)) + v.UI.Noticef("Run '%s' to clean up unreferenced remote blobs.", + pruneCommandHint) } return nil @@ -1069,7 +1078,7 @@ func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error { log.Info("Removing stale local snapshot record", "snapshot_id", id) if !opts.JSON { - v.stdoutf("Removing stale local record: %s\n", id) + v.UI.Infof("Removing stale local record: %s.", v.UI.Snapshot(id)) } err = v.deleteSnapshotFromLocalDB(id) @@ -1092,9 +1101,9 @@ func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error { } if removed == 0 { - v.printlnStdout("No stale local snapshots found.") + v.UI.Infof("No stale local snapshots found.") } else { - v.stdoutf("Removed %d stale local snapshot record(s).\n", removed) + v.UI.Infof("Removed %d stale local snapshot record(s).", removed) } return nil @@ -1273,14 +1282,15 @@ func (v *Vaultik) RemoveSnapshot( } if inLocalIndex { - v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID) + v.UI.Infof("Removed snapshot '%s' from local database.", + v.UI.Snapshot(snapshotID)) } if result.RemoteRemoved { - v.printlnStdout("Removed snapshot metadata from remote storage") - v.stdoutf("\nNote: The removed snapshot's blobs remain on the remote. "+ + v.UI.Infof("Removed snapshot metadata from remote storage.") + v.UI.Noticef("The removed snapshot's blobs remain on the remote. "+ "Run '%s' to delete any blobs no longer referenced by any "+ - "remaining remote snapshot.\n", pruneCommandHint) + "remaining remote snapshot.", pruneCommandHint) } return result, nil @@ -1336,21 +1346,23 @@ func (v *Vaultik) removeSnapshotDryRun( return result, v.outputRemoveJSON(result) } - v.stdoutf("Would remove snapshot: %s\n", snapshotID) + v.UI.Infof("Would remove snapshot: %s.", v.UI.Snapshot(snapshotID)) if !opts.LocalOnly { - v.printlnStdout("Would also remove snapshot metadata from " + - "remote storage (blobs untouched)") + v.UI.Infof("Would also remove snapshot metadata from " + + "remote storage (blobs untouched).") } - v.printlnStdout("[Dry run - no changes made]") + v.UI.Noticef("Dry run: no changes made.") return result, nil } // confirmRemoveSnapshot prompts for confirmation before removing a // snapshot. Returns false (treating EOF/read errors as "no") unless the -// user answers "y". +// user answers "y". The prompt and its answer bypass the UI: they are an +// interactive exchange the operator must see, so --quiet does not +// silence them. func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions) bool { if opts.LocalOnly { v.stdoutf("Remove snapshot '%s' from local database "+ @@ -1456,7 +1468,7 @@ func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error) if len(localSnaps) == 0 && len(orphanRemoteKeys) == 0 { if !opts.JSON { - v.printlnStdout("No snapshots found") + v.UI.Infof("No snapshots found.") } return &RemoveResult{}, nil @@ -1561,27 +1573,27 @@ func (v *Vaultik) handleRemoveAllDryRun( } if !opts.JSON { - v.stdoutf("Would remove %d local snapshot(s):\n", len(localSnaps)) + v.UI.Infof("Would remove %d local snapshot(s):", len(localSnaps)) for _, id := range localSnaps { - v.stdoutf(" %s\n", id) + v.UI.Detailf("%s", v.UI.Snapshot(id)) } if !opts.LocalOnly { if len(orphanRemoteKeys) > 0 { - v.stdoutf("Would also remove %d orphan remote snapshot key(s):\n", + v.UI.Infof("Would also remove %d orphan remote snapshot key(s):", len(orphanRemoteKeys)) for _, key := range orphanRemoteKeys { - v.stdoutf(" %s\n", key) + v.UI.Detailf("%s", key) } } else { - v.printlnStdout("Would also remove snapshot metadata from " + - "remote storage (blobs untouched)") + v.UI.Infof("Would also remove snapshot metadata from " + + "remote storage (blobs untouched).") } } - v.printlnStdout("[Dry run - no changes made]") + v.UI.Noticef("Dry run: no changes made.") } if opts.JSON { @@ -1630,10 +1642,10 @@ func (v *Vaultik) executeRemoveAll( return result, v.outputRemoveJSON(result) } - v.stdoutf("Removed %d snapshot(s)\n", len(result.SnapshotsRemoved)) + v.UI.Infof("Removed %d snapshot(s).", len(result.SnapshotsRemoved)) if !opts.LocalOnly && result.RemoteRemoved { - v.printlnStdout("Removed snapshot metadata from remote storage") + v.UI.Infof("Removed snapshot metadata from remote storage.") } return result, nil