Add atomic writes, humanized sizes, debug logging, and -v/-q per-command
- Atomic writes for mfer gen: writes to temp file, renames on success, cleans up temp on error/interrupt. Prevents empty manifests on Ctrl-C. - Humanized byte sizes using dustin/go-humanize (e.g., "10 MiB" not "10485760") - Progress lines clear when done (using ANSI escape \r\033[K]) - Debug logging when files are added to manifest (mfer gen -vv) - Move -v/-q flags from global to per-command for better UX - Add tests for atomic write behavior with failing filesystem mock
This commit is contained in:
@@ -2,6 +2,7 @@ package cli
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
@@ -67,7 +68,7 @@ func TestGenerateCommand(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0644))
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("test content"), 0644))
|
||||
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
|
||||
exitCode := RunWithOptions(opts)
|
||||
|
||||
@@ -88,12 +89,12 @@ func TestGenerateAndCheckCommand(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/file2.txt", []byte("test content"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String())
|
||||
|
||||
// Check manifest
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 0, exitCode, "check failed: %s", opts.Stderr.(*bytes.Buffer).String())
|
||||
}
|
||||
@@ -106,7 +107,7 @@ func TestCheckCommandWithMissingFile(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String())
|
||||
|
||||
@@ -114,7 +115,7 @@ func TestCheckCommandWithMissingFile(t *testing.T) {
|
||||
require.NoError(t, fs.Remove("/testdir/file1.txt"))
|
||||
|
||||
// Check manifest - should fail
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "check should have failed for missing file")
|
||||
}
|
||||
@@ -127,7 +128,7 @@ func TestCheckCommandWithCorruptedFile(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String())
|
||||
|
||||
@@ -135,7 +136,7 @@ func TestCheckCommandWithCorruptedFile(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("HELLO WORLD"), 0644))
|
||||
|
||||
// Check manifest - should fail with hash mismatch
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "check should have failed for corrupted file")
|
||||
}
|
||||
@@ -148,7 +149,7 @@ func TestCheckCommandWithSizeMismatch(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String())
|
||||
|
||||
@@ -156,7 +157,7 @@ func TestCheckCommandWithSizeMismatch(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("different size content here"), 0644))
|
||||
|
||||
// Check manifest - should fail with size mismatch
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "check should have failed for size mismatch")
|
||||
}
|
||||
@@ -196,7 +197,7 @@ func TestGenerateExcludesDotfilesByDefault(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/.hidden", []byte("secret"), 0644))
|
||||
|
||||
// Generate manifest without --include-dotfiles (default excludes dotfiles)
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
@@ -220,7 +221,7 @@ func TestGenerateWithIncludeDotfiles(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/.hidden", []byte("secret"), 0644))
|
||||
|
||||
// Generate manifest with --include-dotfiles
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "--include-dotfiles", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "--include-dotfiles", "-o", "/testdir/test.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
@@ -240,7 +241,7 @@ func TestMultipleInputPaths(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/dir2/file2.txt", []byte("content2"), 0644))
|
||||
|
||||
// Generate manifest from multiple paths
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/output.mf", "/dir1", "/dir2"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/dir1", "/dir2"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
assert.Equal(t, 0, exitCode, "stderr: %s", opts.Stderr.(*bytes.Buffer).String())
|
||||
|
||||
@@ -257,12 +258,12 @@ func TestNoExtraFilesPass(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("world"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
// Check with --no-extra-files (should pass - no extra files)
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 0, exitCode)
|
||||
}
|
||||
@@ -275,7 +276,7 @@ func TestNoExtraFilesFail(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
@@ -283,7 +284,7 @@ func TestNoExtraFilesFail(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/extra.txt", []byte("extra"), 0644))
|
||||
|
||||
// Check with --no-extra-files (should fail - extra file exists)
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "check should fail when extra files exist")
|
||||
}
|
||||
@@ -297,7 +298,7 @@ func TestNoExtraFilesWithSubdirectory(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/file2.txt", []byte("world"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
@@ -305,7 +306,7 @@ func TestNoExtraFilesWithSubdirectory(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/extra.txt", []byte("extra"), 0644))
|
||||
|
||||
// Check with --no-extra-files (should fail)
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "check should fail when extra files exist in subdirectory")
|
||||
}
|
||||
@@ -318,7 +319,7 @@ func TestCheckWithoutNoExtraFilesIgnoresExtra(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "-q", "generate", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
@@ -326,7 +327,159 @@ func TestCheckWithoutNoExtraFilesIgnoresExtra(t *testing.T) {
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/extra.txt", []byte("extra"), 0644))
|
||||
|
||||
// Check WITHOUT --no-extra-files (should pass - extra files ignored)
|
||||
opts = testOpts([]string{"mfer", "-q", "check", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/manifest.mf"}, fs)
|
||||
exitCode = RunWithOptions(opts)
|
||||
assert.Equal(t, 0, exitCode, "check without --no-extra-files should ignore extra files")
|
||||
}
|
||||
|
||||
func TestGenerateAtomicWriteNoTempFileOnSuccess(t *testing.T) {
|
||||
fs := afero.NewMemMapFs()
|
||||
|
||||
// Create test file
|
||||
require.NoError(t, fs.MkdirAll("/testdir", 0755))
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
// Verify output file exists
|
||||
exists, err := afero.Exists(fs, "/output.mf")
|
||||
require.NoError(t, err)
|
||||
assert.True(t, exists, "output file should exist")
|
||||
|
||||
// Verify temp file does NOT exist
|
||||
tmpExists, err := afero.Exists(fs, "/output.mf.tmp")
|
||||
require.NoError(t, err)
|
||||
assert.False(t, tmpExists, "temp file should not exist after successful generation")
|
||||
}
|
||||
|
||||
func TestGenerateAtomicWriteOverwriteWithForce(t *testing.T) {
|
||||
fs := afero.NewMemMapFs()
|
||||
|
||||
// Create test file
|
||||
require.NoError(t, fs.MkdirAll("/testdir", 0755))
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0644))
|
||||
|
||||
// Create existing manifest with different content
|
||||
require.NoError(t, afero.WriteFile(fs, "/output.mf", []byte("old content"), 0644))
|
||||
|
||||
// Generate manifest with --force
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-f", "-o", "/output.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
// Verify output file exists and was overwritten
|
||||
content, err := afero.ReadFile(fs, "/output.mf")
|
||||
require.NoError(t, err)
|
||||
assert.NotEqual(t, "old content", string(content), "manifest should be overwritten")
|
||||
|
||||
// Verify temp file does NOT exist
|
||||
tmpExists, err := afero.Exists(fs, "/output.mf.tmp")
|
||||
require.NoError(t, err)
|
||||
assert.False(t, tmpExists, "temp file should not exist after successful generation")
|
||||
}
|
||||
|
||||
func TestGenerateFailsWithoutForceWhenOutputExists(t *testing.T) {
|
||||
fs := afero.NewMemMapFs()
|
||||
|
||||
// Create test file
|
||||
require.NoError(t, fs.MkdirAll("/testdir", 0755))
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0644))
|
||||
|
||||
// Create existing manifest
|
||||
require.NoError(t, afero.WriteFile(fs, "/output.mf", []byte("existing"), 0644))
|
||||
|
||||
// Generate manifest WITHOUT --force (should fail)
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "should fail when output exists without --force")
|
||||
|
||||
// Verify original content is preserved
|
||||
content, err := afero.ReadFile(fs, "/output.mf")
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "existing", string(content), "original file should be preserved")
|
||||
}
|
||||
|
||||
func TestGenerateAtomicWriteUsesTemp(t *testing.T) {
|
||||
// This test verifies that generate uses a temp file by checking
|
||||
// that the output file doesn't exist until generation completes.
|
||||
// We do this by generating to a path and verifying the temp file
|
||||
// pattern is used (output.mf.tmp -> output.mf)
|
||||
fs := afero.NewMemMapFs()
|
||||
|
||||
// Create test file
|
||||
require.NoError(t, fs.MkdirAll("/testdir", 0755))
|
||||
require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0644))
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
// Both output file should exist and temp should not
|
||||
exists, _ := afero.Exists(fs, "/output.mf")
|
||||
assert.True(t, exists, "output file should exist")
|
||||
|
||||
tmpExists, _ := afero.Exists(fs, "/output.mf.tmp")
|
||||
assert.False(t, tmpExists, "temp file should be cleaned up")
|
||||
|
||||
// Verify manifest is valid (not empty)
|
||||
content, err := afero.ReadFile(fs, "/output.mf")
|
||||
require.NoError(t, err)
|
||||
assert.True(t, len(content) > 0, "manifest should not be empty")
|
||||
}
|
||||
|
||||
// failingWriterFs wraps a filesystem and makes writes fail after N bytes
|
||||
type failingWriterFs struct {
|
||||
afero.Fs
|
||||
failAfter int64
|
||||
written int64
|
||||
}
|
||||
|
||||
type failingFile struct {
|
||||
afero.File
|
||||
fs *failingWriterFs
|
||||
}
|
||||
|
||||
func (f *failingFile) Write(p []byte) (int, error) {
|
||||
f.fs.written += int64(len(p))
|
||||
if f.fs.written > f.fs.failAfter {
|
||||
return 0, fmt.Errorf("simulated write failure")
|
||||
}
|
||||
return f.File.Write(p)
|
||||
}
|
||||
|
||||
func (fs *failingWriterFs) Create(name string) (afero.File, error) {
|
||||
f, err := fs.Fs.Create(name)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &failingFile{File: f, fs: fs}, nil
|
||||
}
|
||||
|
||||
func TestGenerateAtomicWriteCleansUpOnError(t *testing.T) {
|
||||
baseFs := afero.NewMemMapFs()
|
||||
|
||||
// Create test files - need enough content to trigger the write failure
|
||||
require.NoError(t, baseFs.MkdirAll("/testdir", 0755))
|
||||
require.NoError(t, afero.WriteFile(baseFs, "/testdir/file1.txt", []byte("hello world this is a test file"), 0644))
|
||||
|
||||
// Wrap with failing writer that fails after writing some bytes
|
||||
fs := &failingWriterFs{Fs: baseFs, failAfter: 10}
|
||||
|
||||
// Generate manifest - should fail during write
|
||||
opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs)
|
||||
exitCode := RunWithOptions(opts)
|
||||
assert.Equal(t, 1, exitCode, "should fail due to write error")
|
||||
|
||||
// With atomic writes: output.mf should NOT exist (temp was cleaned up)
|
||||
// With non-atomic writes: output.mf WOULD exist (partial/empty)
|
||||
exists, _ := afero.Exists(baseFs, "/output.mf")
|
||||
assert.False(t, exists, "output file should not exist after failed generation (atomic write)")
|
||||
|
||||
// Temp file should also not exist
|
||||
tmpExists, _ := afero.Exists(baseFs, "/output.mf.tmp")
|
||||
assert.False(t, tmpExists, "temp file should be cleaned up after failed generation")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user