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:27:01 +00:00
parent 2322f8bb02
commit 5d8862938d
2 changed files with 49 additions and 23 deletions
+6
View File
@@ -29,6 +29,12 @@
# Completed Steps # Completed Steps
- the three disk size tests in `scan_test.go` no longer fail at random on ZFS,
where `unix.Sync` can return before a file's blocks are counted. 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)
- `README.md` rewritten from scratch, sentence by sentence, as a newcomer reads - `README.md` rewritten from scratch, sentence by sentence, as a newcomer reads
it. It leads with what the program is and how to use it. A reference section it. It leads with what the program is and how to use it. A reference section
covers every setting and every subcommand. Development and testing come last. covers every setting and every subcommand. Development and testing come last.
+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)
} }
} }