check always warns about files the manifest does not list (closes #103)
check / check (push) Failing after 2s
check / check (push) Failing after 2s
check now always looks under the base directory for files the manifest does not list, hidden files and directories included, and prints one warning per file. The result still depends only on the listed files; --no-extra-files turns each unlisted file into a failure, and --quiet hides the warnings but not the failures. A directory that cannot be searched is likewise only a warning unless that flag is given. The manifest itself is left out by file identity, as gen and freshen do, so no name or spelling of its path or of the base gets it reported. Model: opus-5-5
This commit is contained in:
+17
-9
@@ -206,28 +206,33 @@ func countCheckFailures(
|
||||
}
|
||||
|
||||
// findExtraFiles reports files present on disk but absent from the
|
||||
// manifest, counting each as a failure.
|
||||
// manifest: each is a failure under --no-extra-files, otherwise a warning.
|
||||
func findExtraFiles(ctx *cli.Context, chk *mfer.Checker, failures *int64) error {
|
||||
extraResults := make(chan mfer.Result, 1)
|
||||
extraDone := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
for result := range extraResults {
|
||||
*failures++
|
||||
if ctx.Bool("no-extra-files") {
|
||||
*failures++
|
||||
|
||||
log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message)
|
||||
log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message)
|
||||
} else {
|
||||
log.Warnf("%s: %s (%s)", result.Status, result.Path, result.Message)
|
||||
}
|
||||
}
|
||||
|
||||
close(extraDone)
|
||||
}()
|
||||
|
||||
err := chk.FindExtraFiles(ctx.Context, extraResults)
|
||||
|
||||
<-extraDone
|
||||
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to check for extra files: %w", err)
|
||||
}
|
||||
|
||||
<-extraDone
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -270,12 +275,15 @@ func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, er
|
||||
// Wait for results processing to complete
|
||||
<-done
|
||||
|
||||
// Check for extra files if requested
|
||||
if ctx.Bool("no-extra-files") {
|
||||
err = findExtraFiles(ctx, chk, &failures)
|
||||
if err != nil {
|
||||
// Without --no-extra-files the result depends only on the files the
|
||||
// manifest lists, so failing to look for others is only a warning.
|
||||
err = findExtraFiles(ctx, chk, &failures)
|
||||
if err != nil {
|
||||
if ctx.Bool("no-extra-files") {
|
||||
return 0, err
|
||||
}
|
||||
|
||||
log.Warn(err.Error())
|
||||
}
|
||||
|
||||
return failures, nil
|
||||
|
||||
+136
-16
@@ -698,30 +698,150 @@ func TestNoExtraFilesWithSubdirectory(t *testing.T) {
|
||||
"check should fail when extra files exist in subdirectory")
|
||||
}
|
||||
|
||||
func TestCheckWithoutNoExtraFilesIgnoresExtra(t *testing.T) {
|
||||
// TestCheckWarnsAboutUnlistedFiles adds one file the manifest does not list
|
||||
// to a tree that had none: it gets one warning and the check passes, unless
|
||||
// --no-extra-files makes it a failure. --quiet hides the warning, not the
|
||||
// failure.
|
||||
func TestCheckWarnsAboutUnlistedFiles(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
for name, unlisted := range map[string]string{
|
||||
"regular file": "extra.txt",
|
||||
"dotfile": ".hidden",
|
||||
"file in hidden directory": ".git/config",
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create test file
|
||||
require.NoError(t, fs.MkdirAll(testDir, 0o755))
|
||||
writeTestFile(t, fs, testFile1, "hello")
|
||||
fs := afero.NewMemMapFs()
|
||||
require.NoError(t, fs.MkdirAll(testDir, 0o755))
|
||||
writeTestFile(t, fs, testFile1, "hello")
|
||||
|
||||
// Generate manifest
|
||||
opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs)
|
||||
exitCode := runCLI(opts)
|
||||
require.Equal(t, 0, exitCode)
|
||||
opts := testOpts([]string{
|
||||
testApp, cmdGenerate, "-q", "-o", testManifest, testDir,
|
||||
}, fs)
|
||||
require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
|
||||
|
||||
// Add extra file
|
||||
writeTestFile(t, fs, "/testdir/extra.txt", "extra")
|
||||
check := func(flags ...string) (int, string) {
|
||||
args := append([]string{testApp, cmdCheck, testFlagBase, testDir}, flags...)
|
||||
opts := testOpts(append(args, testManifest), fs)
|
||||
|
||||
return runCLI(opts), testStderr(t, opts)
|
||||
}
|
||||
|
||||
exitCode, stderr := check()
|
||||
assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
|
||||
assert.NotContains(t, stderr, "not in manifest")
|
||||
|
||||
writeTestFile(t, fs, filepath.Join(testDir, unlisted), "unlisted")
|
||||
|
||||
exitCode, stderr = check()
|
||||
assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
|
||||
assert.Equal(t, 1, strings.Count(stderr, "not in manifest"), stderr)
|
||||
assert.Contains(t, stderr, unlisted)
|
||||
|
||||
exitCode, stderr = check("-q")
|
||||
assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
|
||||
assert.NotContains(t, stderr, "not in manifest")
|
||||
|
||||
exitCode, stderr = check(testFlagNoExtra)
|
||||
assert.Equal(t, 1, exitCode, "stderr: %s", stderr)
|
||||
assert.Contains(t, stderr, unlisted)
|
||||
|
||||
exitCode, _ = check("-q", testFlagNoExtra)
|
||||
assert.Equal(t, 1, exitCode)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckNeverReportsManifest keeps the manifest inside the checked tree,
|
||||
// under several names and path spellings, and names the tree both directly
|
||||
// and through a symlink: the manifest is never reported, even under
|
||||
// --no-extra-files. The manifest is recognized by file identity, which needs
|
||||
// the real filesystem.
|
||||
func TestCheckNeverReportsManifest(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Manifest paths are relative to the checked tree.
|
||||
for name, manifest := range map[string]string{
|
||||
"default name": defaultManifestName,
|
||||
"hidden name": ".index.mf",
|
||||
"other name in a subdirectory": "sub/listing.mf",
|
||||
"path spelled through ..": "sub/../index.mf",
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A temp dir holding data/tree and link, a symlink to data.
|
||||
root := t.TempDir()
|
||||
tree := filepath.Join(root, "data", "tree")
|
||||
// Not filepath.Join, which would clean away a "..".
|
||||
manifestPath := tree + "/" + manifest
|
||||
|
||||
fs := afero.NewOsFs()
|
||||
require.NoError(t, fs.MkdirAll(filepath.Join(tree, "sub"), 0o750))
|
||||
require.NoError(t,
|
||||
os.Symlink(filepath.Join(root, "data"), filepath.Join(root, "link")))
|
||||
writeTestFile(t, fs, filepath.Join(tree, testFileTxt), "hello")
|
||||
|
||||
opts := testOpts([]string{
|
||||
testApp, cmdGenerate, "-q", "-o", manifestPath, tree,
|
||||
}, fs)
|
||||
require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
|
||||
|
||||
for _, base := range []string{tree, filepath.Join(root, "link", "tree")} {
|
||||
opts = testOpts([]string{
|
||||
testApp, cmdCheck, testFlagNoExtra, testFlagBase, base, manifestPath,
|
||||
}, fs)
|
||||
assert.Equal(t, 0, runCLI(opts),
|
||||
"base %s, stderr: %s", base, testStderr(t, opts))
|
||||
assert.NotContains(t, testStderr(t, opts), "not in manifest")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// unlistableDirFs is a filesystem on which one directory cannot be listed.
|
||||
type unlistableDirFs struct {
|
||||
afero.Fs
|
||||
|
||||
dir string
|
||||
}
|
||||
|
||||
//nolint:ireturn // Open must return afero.File to satisfy afero.Fs.
|
||||
func (f unlistableDirFs) Open(name string) (afero.File, error) {
|
||||
if name == f.dir {
|
||||
return nil, os.ErrPermission
|
||||
}
|
||||
|
||||
return f.Fs.Open(name)
|
||||
}
|
||||
|
||||
// TestCheckWithUnlistableDirectory has a directory under the base that
|
||||
// cannot be listed: the check still passes, with a warning, unless
|
||||
// --no-extra-files is given.
|
||||
func TestCheckWithUnlistableDirectory(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mem := afero.NewMemMapFs()
|
||||
require.NoError(t, mem.MkdirAll("/testdir/locked", 0o755))
|
||||
writeTestFile(t, mem, testFile1, "hello")
|
||||
|
||||
opts := testOpts([]string{
|
||||
testApp, cmdGenerate, "-q", "-o", testManifest, testDir,
|
||||
}, mem)
|
||||
require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
|
||||
|
||||
fs := unlistableDirFs{Fs: mem, dir: "/testdir/locked"}
|
||||
|
||||
opts = testOpts([]string{testApp, cmdCheck, testFlagBase, testDir, testManifest}, fs)
|
||||
assert.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
|
||||
assert.Contains(t, testStderr(t, opts), os.ErrPermission.Error())
|
||||
|
||||
// Check WITHOUT --no-extra-files (should pass - extra files ignored)
|
||||
opts = testOpts([]string{
|
||||
testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest,
|
||||
testApp, cmdCheck, testFlagNoExtra, testFlagBase, testDir, testManifest,
|
||||
}, fs)
|
||||
exitCode = runCLI(opts)
|
||||
assert.Equal(t, 0, exitCode,
|
||||
"check without --no-extra-files should ignore extra files")
|
||||
assert.Equal(t, 1, runCLI(opts), "stderr: %s", testStderr(t, opts))
|
||||
}
|
||||
|
||||
func TestGenerateAtomicWriteNoTempFileOnSuccess(t *testing.T) {
|
||||
|
||||
@@ -227,7 +227,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
|
||||
},
|
||||
&cli.BoolFlag{
|
||||
Name: "no-extra-files",
|
||||
Usage: "Fail if files exist in base directory that are not in manifest",
|
||||
Usage: "Fail, instead of warning, if files in base directory are not in manifest",
|
||||
},
|
||||
&cli.StringFlag{
|
||||
Name: "require-signature",
|
||||
|
||||
Reference in New Issue
Block a user