diff --git a/TODO.md b/TODO.md index 19e271f..6f87fb5 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,13 @@ Newest first. An entry ends with its date and its issue or branch, where those are known. +- The three disk size tests in `scan_test.go` no longer fail at random on ZFS. + `unix.Sync` can return before ZFS has counted a file's blocks, and one ZFS + commit can take a file through several sizes. The test that needs the size to + change waits for it, for at most 30 seconds. Each test reads the size before + and after its scan. It fails only on a `disk_size` outside those two reads + (2026-11-02, https://git.eeqj.de/sneak/sfdupes/issues/150) + - `TODO.md` rewritten from scratch by the same rules as `README.md`, sentence by sentence. Every completed step is kept, with its date and its issue or branch. The policy-compliance checklist of 2026-07-22 is folded into its step diff --git a/scan_test.go b/scan_test.go index 42ba0d1..5e3b2bf 100644 --- a/scan_test.go +++ b/scan_test.go @@ -43,10 +43,15 @@ func writeFile(t *testing.T, dir, name string, data []byte) string { } // diskSizeOf returns the bytes the file at path takes on disk, from its -// own lstat: st_blocks counts 512-byte units. ZFS counts a new file's -// blocks only seconds after the write, and fsync does not hasten that. -// A test therefore calls unix.Sync, which waits for it, before the scan -// whose disk_size it compares with this. +// own lstat: st_blocks counts 512-byte units. ZFS counts a write's +// blocks only when it commits the write, up to seconds later, and one +// commit can take the file through several growing sizes. So this value +// can change at any moment. Neither fsync nor unix.Sync hastens that, +// but a unix.Sync call often takes long enough that the blocks are +// counted when it returns. A test that needs the value to change waits +// for the change. A test that compares it with a scan's disk_size reads +// it before and after the scan, and fails only on a disk_size outside +// those two reads. func diskSizeOf(t *testing.T, path string) int64 { t.Helper() @@ -1448,20 +1453,26 @@ func TestScanStoresDiskSize(t *testing.T) { unix.Sync() // see diskSizeOf - want := diskSizeOf(t, p) - if want == 0 || want >= length { + size := diskSizeOf(t, p) + if size == 0 || size >= length { t.Fatalf("the file takes %d bytes on disk, want more than none "+ - "and fewer than its length %d", want, length) + "and fewer than its length %d", size, length) } for _, operand := range []string{filepath.Dir(p), p} { db := openTestDB(t) + + // ZFS may count the blocks during the scan; see diskSizeOf. + before := diskSizeOf(t, p) + syncTree(t, db, operand) + after := diskSizeOf(t, p) + got := recordByPath(t, dbRecords(t, db), p).diskSize - if got != want { - t.Errorf("scan of %s: disk_size = %d, want %d", - operand, got, want) + if got < before || got > after { + t.Errorf("scan of %s: disk_size = %d, want %d to %d", + operand, got, before, after) } } } @@ -1479,7 +1490,7 @@ func TestSyncScanRewriteUpdatesDiskSize(t *testing.T) { syncTree(t, db, dir) - before := recordByPath(t, dbRecords(t, db), p).diskSize + recorded := recordByPath(t, dbRecords(t, db), p).diskSize pokeAt(t, p, 512*1024, pattern(7, 64*1024)) @@ -1492,22 +1503,33 @@ func TestSyncScanRewriteUpdatesDiskSize(t *testing.T) { t.Fatal(err) } - unix.Sync() // see diskSizeOf + // Wait for ZFS to count the write; see diskSizeOf. + deadline := time.Now().Add(30 * time.Second) + for diskSizeOf(t, p) == recorded { + if time.Now().After(deadline) { + t.Fatalf("the file still takes %d bytes on disk 30 seconds "+ + "after the write into its hole", recorded) + } - want := diskSizeOf(t, p) - if want == before { - t.Fatalf("the file still takes %d bytes on disk after the write "+ - "into its hole", want) + unix.Sync() } + // ZFS may still be counting, so the size may change during the rescan. + // A rescan that keeps the old size still fails, because the wait made + // before differ from it. + before := diskSizeOf(t, p) + st := syncTree(t, db, dir) + + after := diskSizeOf(t, p) + if st != (scanStats{walked: 1, updated: 1}) { t.Fatalf("rescan stats = %+v, want 1 updated", st) } got := recordByPath(t, dbRecords(t, db), p).diskSize - if got != want { - t.Errorf("disk_size = %d, want %d", got, want) + if got < before || got > after { + t.Errorf("disk_size = %d, want %d to %d", got, before, after) } } @@ -1533,15 +1555,21 @@ func TestSyncScanUnchangedUpdatesDiskSize(t *testing.T) { unix.Sync() // see diskSizeOf + // ZFS may count the blocks during the rescan; see diskSizeOf. + before := diskSizeOf(t, p) + // A file read again would count as updated. st := syncTree(t, db, dir) + + after := diskSizeOf(t, p) + if st != (scanStats{walked: 1, unchanged: 1}) { t.Fatalf("rescan stats = %+v, want 1 unchanged", st) } got := recordByPath(t, dbRecords(t, db), p).diskSize - if want := diskSizeOf(t, p); got != want { - t.Errorf("disk_size = %d, want %d", got, want) + if got < before || got > after { + t.Errorf("disk_size = %d, want %d to %d", got, before, after) } }