Hold runMu in internal/cli tests that log outside a CLI run (closes #210)

A CLI run under runMu points the process-global logger at its own
buffers, so a line another test logged outside runMu could land in that
run's output. TestCheckQuietShowsFailedFiles caught one: a "loaded
manifest" line in the stderr of a passing check -q run.

The test helpers that scan or load a manifest now hold runMu
themselves, and so do the remaining direct loads and scans, the
recordEntry call and the retrying download. Two dotfile tests use
manifestPaths instead of loading the manifest themselves.

Judgement call: a downloadFile call that gets its answer on the first
request does not log, so those calls are left as they are.

Model: opus-5-5

internal/cli/entry_test.go

@@ -771,13 +771,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 +794,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 +1215,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,

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))
 

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))

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(),
+	// 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]))
 	}
 
+	var manifest bytes.Buffer
+
+	// 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))
 
-	var manifest bytes.Buffer
-	require.NoError(t, scanner.ToManifest(context.Background(), &manifest, nil))
+		return scanner.ToManifest(context.Background(), &manifest, nil)
+	}))
 
 	tree := fetchTestHandler(manifest.Bytes(), files)
 

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)