diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index 600e571..2d35d92 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -58,9 +58,10 @@ func runCLI(opts *RunOptions) int { // runCLIWithLevel is runCLI that also returns the log level the run left // set, read while runMu still keeps other runs from changing it. Each run // starts at the default level, as a new process does. Before releasing the -// lock it points the process-global logger at io.Discard: other tests log -// outside the lock (manifest loads, scans), and those lines must not land in -// this run's buffers once it has returned and its test is reading them. +// lock it points the process-global logger at io.Discard: test helpers also +// log under runMu outside any run (manifest loads, scans), and those lines +// must not land in this run's buffers once it has returned and its test is +// reading them. func runCLIWithLevel(opts *RunOptions) (int, log.Level) { runMu.Lock() defer runMu.Unlock() @@ -206,13 +207,13 @@ func TestVersionFlagAndCommandMatch(t *testing.T) { // TestRootVerbosityFlags asserts that -q and -v given before any subcommand // name take effect: in the root itself, and in the fetch and export -// subcommands (issue #64). It does not run in parallel: other tests log -// outside runMu (manifest loads, scans), and a line logged while one of these -// runs is in progress lands in its output. -// -//nolint:paralleltest // see above +// subcommands (issue #64). func TestRootVerbosityFlags(t *testing.T) { + t.Parallel() + t.Run("quiet with no subcommand hides the banner", func(t *testing.T) { + t.Parallel() + loud := testOpts([]string{testApp}, afero.NewMemMapFs()) require.Equal(t, 0, runCLI(loud)) assert.Contains(t, testStdout(t, loud), banner) @@ -224,6 +225,8 @@ func TestRootVerbosityFlags(t *testing.T) { }) t.Run("quiet before fetch hides the banner", func(t *testing.T) { + t.Parallel() + opts := testOpts([]string{testApp, "-q", cmdFetch}, afero.NewMemMapFs()) assert.Equal(t, 1, runCLI(opts)) @@ -232,6 +235,8 @@ func TestRootVerbosityFlags(t *testing.T) { }) t.Run("verbose twice before fetch enables debug logging", func(t *testing.T) { + t.Parallel() + opts := testOpts([]string{testApp, "-v", "-v", cmdFetch}, afero.NewMemMapFs()) assert.Equal(t, 1, runCLI(opts)) @@ -239,6 +244,8 @@ func TestRootVerbosityFlags(t *testing.T) { }) t.Run("quiet before export hides the manifest summary", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() manifest := buildTestManifest(t, map[string][]byte{"a.txt": []byte("a")}) require.NoError(t, afero.WriteFile(fs, testManifest, manifest, 0o644)) @@ -771,13 +778,9 @@ func TestGenerateExcludesDotfilesByDefault(t *testing.T) { assert.True(t, exists) // Verify manifest only has 1 file (the non-dotfile) - manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{ - Path: testMF, - Fs: fs, - }) - require.NoError(t, err) - assert.Len(t, manifest.Files(), 1) - assert.Equal(t, "file1.txt", manifest.Files()[0].GetPath()) + paths := manifestPaths(t, fs, testMF) + require.Len(t, paths, 1) + assert.Equal(t, "file1.txt", paths[0]) } func TestGenerateWithIncludeDotfiles(t *testing.T) { @@ -798,12 +801,7 @@ func TestGenerateWithIncludeDotfiles(t *testing.T) { require.Equal(t, 0, exitCode) // Verify manifest has 2 files (including dotfile) - manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{ - Path: testMF, - Fs: fs, - }) - require.NoError(t, err) - assert.Len(t, manifest.Files(), 2) + assert.Len(t, manifestPaths(t, fs, testMF), 2) } func TestMultipleInputPaths(t *testing.T) { @@ -1224,6 +1222,11 @@ func TestGenerateFailsWithoutForceWhenOutputExists(t *testing.T) { func manifestPaths(t *testing.T, fs afero.Fs, path string) []string { t.Helper() + // Loading a manifest logs to the process-global logger, so serialize + // with the CLI runs. + runMu.Lock() + defer runMu.Unlock() + manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{ Path: path, Fs: fs, diff --git a/internal/cli/errmsg_test.go b/internal/cli/errmsg_test.go index e3dd9c6..8f8e817 100644 --- a/internal/cli/errmsg_test.go +++ b/internal/cli/errmsg_test.go @@ -54,6 +54,11 @@ func runLocked(fn func() error) error { func unsignedChecker(t *testing.T) *mfer.Checker { t.Helper() + // The scan and the manifest load log to the process-global logger, so + // serialize with the CLI runs. + runMu.Lock() + defer runMu.Unlock() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/d", 0o755)) require.NoError(t, afero.WriteFile(fs, "/d/f.txt", []byte("hi"), 0o644)) @@ -183,6 +188,11 @@ func signedManifest(t *testing.T, files map[string][]byte) []byte { func signedChecker(t *testing.T, manifest []byte) *mfer.Checker { t.Helper() + // Loading a manifest logs to the process-global logger, so serialize + // with the CLI runs. + runMu.Lock() + defer runMu.Unlock() + fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/index.mf", manifest, 0o644)) diff --git a/internal/cli/export_test.go b/internal/cli/export_test.go index 8bb700c..ac76bc2 100644 --- a/internal/cli/export_test.go +++ b/internal/cli/export_test.go @@ -20,6 +20,11 @@ const testCmdExport = "export" func buildTestManifest(t *testing.T, files map[string][]byte) []byte { t.Helper() + // The scan logs to the process-global logger, so serialize with the + // CLI runs. + runMu.Lock() + defer runMu.Unlock() + sourceFs := afero.NewMemMapFs() for path, content := range files { require.NoError(t, sourceFs.MkdirAll("/", 0o755)) diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index 98b5bdc..fa773bd 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -171,6 +171,11 @@ func TestResolveManifestURL(t *testing.T) { func scanToManifest(t *testing.T, sourceFs afero.Fs) []byte { t.Helper() + // The scan logs to the process-global logger, so serialize with the + // CLI runs. + runMu.Lock() + defer runMu.Unlock() + s := mfer.NewScannerWithOptions(&mfer.ScannerOptions{Fs: sourceFs}) require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) @@ -314,8 +319,11 @@ func TestFetchFromHTTP(t *testing.T) { // Change to a fresh destination directory for the test destDir := chdirTemp(t) - // Parse the manifest to get file entries + // Parse the manifest to get file entries. Loading a manifest logs to + // the process-global logger. + runMu.Lock() manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) + runMu.Unlock() require.NoError(t, err) files := manifest.Files() @@ -365,7 +373,10 @@ func TestFetchHashMismatch(t *testing.T) { // Generate and parse manifest manifestData := scanToManifest(t, sourceFs) + // Loading a manifest logs to the process-global logger. + runMu.Lock() manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) + runMu.Unlock() require.NoError(t, err) files := manifest.Files() @@ -411,7 +422,10 @@ func TestFetchSizeMismatch(t *testing.T) { // Generate and parse manifest manifestData := scanToManifest(t, sourceFs) + // Loading a manifest logs to the process-global logger. + runMu.Lock() manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) + runMu.Unlock() require.NoError(t, err) files := manifest.Files() @@ -523,7 +537,10 @@ func TestFetchProgress(t *testing.T) { // Generate and parse manifest manifestData := scanToManifest(t, sourceFs) + // Loading a manifest logs to the process-global logger. + runMu.Lock() manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) + runMu.Unlock() require.NoError(t, err) files := manifest.Files() @@ -897,9 +914,12 @@ func TestGetStopsWaitingWhenCanceled(t *testing.T) { //nolint:paralleltest // changes the process-global working directory func TestDownloadFileRetriesToSuccess(t *testing.T) { content := []byte("the whole file, every byte of it") + manifestData := manifestOf(t, map[string][]byte{testFileTxt: content}) - manifest, err := mfer.NewManifestFromReader(bytes.NewReader( - manifestOf(t, map[string][]byte{testFileTxt: content}))) + // Loading a manifest logs to the process-global logger. + runMu.Lock() + manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) + runMu.Unlock() require.NoError(t, err) var requests atomic.Int32 @@ -922,8 +942,11 @@ func TestDownloadFileRetriesToSuccess(t *testing.T) { chdirTemp(t) - err = downloadFile(context.Background(), testClient(), - server.URL+"/"+testFileTxt, ".", testFileTxt, manifest.Files()[0], nil) + // downloadFile logs each retry to the process-global logger. + err = runLocked(func() error { + return downloadFile(context.Background(), testClient(), + server.URL+"/"+testFileTxt, ".", testFileTxt, manifest.Files()[0], nil) + }) require.NoError(t, err) assert.Equal(t, int32(3), requests.Load()) @@ -1404,14 +1427,18 @@ func TestFetchSetsRecordedMode(t *testing.T) { require.NoError(t, afero.WriteFile(sourceFs, "/"+p, content, modes[p])) } - scanner := mfer.NewScannerWithOptions(&mfer.ScannerOptions{ - Fs: sourceFs, - IncludePermissions: true, - }) - require.NoError(t, scanner.EnumerateFS(sourceFs, "/", nil)) - var manifest bytes.Buffer - require.NoError(t, scanner.ToManifest(context.Background(), &manifest, nil)) + + // The scan logs to the process-global logger. + require.NoError(t, runLocked(func() error { + scanner := mfer.NewScannerWithOptions(&mfer.ScannerOptions{ + Fs: sourceFs, + IncludePermissions: true, + }) + require.NoError(t, scanner.EnumerateFS(sourceFs, "/", nil)) + + return scanner.ToManifest(context.Background(), &manifest, nil) + })) tree := fetchTestHandler(manifest.Bytes(), files) diff --git a/internal/cli/freshen_test.go b/internal/cli/freshen_test.go index bde7f52..a4ab325 100644 --- a/internal/cli/freshen_test.go +++ b/internal/cli/freshen_test.go @@ -70,6 +70,11 @@ func runFreshen(t *testing.T, fs afero.Fs, root, manifestPath string) { func manifestFiles(t *testing.T, fs afero.Fs, path string) []*mfer.MFFilePath { t.Helper() + // Loading a manifest logs to the process-global logger, so serialize + // with the CLI runs. + runMu.Lock() + defer runMu.Unlock() + manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{ Path: path, Fs: fs, @@ -436,7 +441,11 @@ func TestFreshenRecordEntryMtimePresence(t *testing.T) { s := &freshenScanner{ existingByPath: map[string]*mfer.MFFilePath{relPath: tc.entry}, } + + // recordEntry logs to the process-global logger. + runMu.Lock() s.recordEntry(relPath, info) + runMu.Unlock() require.Len(t, s.entries, 1) assert.Equal(t, tc.needsHash, s.entries[0].needsHash)