Stop the disk size tests failing at random on ZFS (closes #150)
check / check (push) Canceled after 0s

unix.Sync can return before ZFS has counted a file's blocks, so a test
that expected the size on disk to be settled after it failed at random.
The test that needs the size to change now calls unix.Sync in a loop
until it does, for at most 30 seconds. All three tests read the size
before and after the scan and accept either value.

Model: opus-5-5
This commit is contained in:
2026-11-02 22:32:07 +00:00
parent 4ecf8dc484
commit d58e9fe256
2 changed files with 49 additions and 23 deletions
+6
View File
@@ -29,6 +29,12 @@
Newest first. An entry ends with its date and its issue or branch, where those Newest first. An entry ends with its date and its issue or branch, where those
are known. 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 - `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. 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 The policy-compliance checklist of 2026-07-22 is folded into its step
+43 -23
View File
@@ -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 // 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 // own lstat: st_blocks counts 512-byte units. ZFS counts a write's
// blocks only seconds after the write, and fsync does not hasten that. // blocks only when it commits the write, seconds later and sometimes in
// A test therefore calls unix.Sync, which waits for it, before the scan // steps, so this value can change at any moment. fsync does not hasten
// whose disk_size it compares with this. // 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 { func diskSizeOf(t *testing.T, path string) int64 {
t.Helper() t.Helper()
@@ -1446,22 +1449,26 @@ func TestScanStoresDiskSize(t *testing.T) {
p := sparseFile(t, t.TempDir(), "sparse", length) p := sparseFile(t, t.TempDir(), "sparse", length)
pokeAt(t, p, 0, []byte("x")) pokeAt(t, p, 0, []byte("x"))
unix.Sync() // see diskSizeOf size := diskSizeOf(t, p)
if size == 0 || size >= length {
want := diskSizeOf(t, p)
if want == 0 || want >= length {
t.Fatalf("the file takes %d bytes on disk, want more than none "+ 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} { for _, operand := range []string{filepath.Dir(p), p} {
db := openTestDB(t) db := openTestDB(t)
// ZFS may count the blocks during the scan; see diskSizeOf.
before := diskSizeOf(t, p)
syncTree(t, db, operand) syncTree(t, db, operand)
after := diskSizeOf(t, p)
got := recordByPath(t, dbRecords(t, db), p).diskSize got := recordByPath(t, dbRecords(t, db), p).diskSize
if got != want { if got != before && got != after {
t.Errorf("scan of %s: disk_size = %d, want %d", t.Errorf("scan of %s: disk_size = %d, want %d or %d",
operand, got, want) operand, got, before, after)
} }
} }
} }
@@ -1479,7 +1486,7 @@ func TestSyncScanRewriteUpdatesDiskSize(t *testing.T) {
syncTree(t, db, dir) 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)) pokeAt(t, p, 512*1024, pattern(7, 64*1024))
@@ -1492,22 +1499,31 @@ func TestSyncScanRewriteUpdatesDiskSize(t *testing.T) {
t.Fatal(err) 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) unix.Sync()
if want == before {
t.Fatalf("the file still takes %d bytes on disk after the write "+
"into its hole", want)
} }
// ZFS may still be counting, so the size may change during the rescan.
before := diskSizeOf(t, p)
st := syncTree(t, db, dir) st := syncTree(t, db, dir)
after := diskSizeOf(t, p)
if st != (scanStats{walked: 1, updated: 1}) { if st != (scanStats{walked: 1, updated: 1}) {
t.Fatalf("rescan stats = %+v, want 1 updated", st) t.Fatalf("rescan stats = %+v, want 1 updated", st)
} }
got := recordByPath(t, dbRecords(t, db), p).diskSize got := recordByPath(t, dbRecords(t, db), p).diskSize
if got != want { if got != before && got != after {
t.Errorf("disk_size = %d, want %d", got, want) t.Errorf("disk_size = %d, want %d or %d", got, before, after)
} }
} }
@@ -1531,17 +1547,21 @@ func TestSyncScanUnchangedUpdatesDiskSize(t *testing.T) {
t.Fatal(err) 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. // A file read again would count as updated.
st := syncTree(t, db, dir) st := syncTree(t, db, dir)
after := diskSizeOf(t, p)
if st != (scanStats{walked: 1, unchanged: 1}) { if st != (scanStats{walked: 1, unchanged: 1}) {
t.Fatalf("rescan stats = %+v, want 1 unchanged", st) t.Fatalf("rescan stats = %+v, want 1 unchanged", st)
} }
got := recordByPath(t, dbRecords(t, db), p).diskSize got := recordByPath(t, dbRecords(t, db), p).diskSize
if want := diskSizeOf(t, p); got != want { if got != before && got != after {
t.Errorf("disk_size = %d, want %d", got, want) t.Errorf("disk_size = %d, want %d or %d", got, before, after)
} }
} }