diff --git a/TODO.md b/TODO.md index 19e271f..b160a62 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,12 @@ 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. 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 and accepts either value (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..b872449 100644 --- a/scan_test.go +++ b/scan_test.go @@ -43,10 +43,13 @@ 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, seconds later and sometimes in +// steps, so this value can change at any moment. fsync does not hasten +// that. unix.Sync usually does, but it can return before the blocks are +// counted. A test that needs the value to change therefore waits for +// that, and a test that compares it with a scan's disk_size reads it +// before and after the scan and accepts either. func diskSizeOf(t *testing.T, path string) int64 { t.Helper() @@ -1446,22 +1449,26 @@ func TestScanStoresDiskSize(t *testing.T) { p := sparseFile(t, t.TempDir(), "sparse", length) pokeAt(t, p, 0, []byte("x")) - 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 or %d", + operand, got, before, after) } } } @@ -1479,7 +1486,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 +1499,31 @@ 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. + 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 or %d", got, before, after) } } @@ -1531,17 +1547,21 @@ func TestSyncScanUnchangedUpdatesDiskSize(t *testing.T) { t.Fatal(err) } - 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 or %d", got, before, after) } }