Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ against a scratch tree and assert on the hashfile and on-disk sharing. Dedupe
cases need a reflink fs (`DUPEREMOVE_TEST_DIR`, set by `devenv.sh`). Keep tests
in `tests/`; no shell tests.

- **The suite runs in parallel by default** (`make integration TEST_JOBS=…`,
`tests/run.py -j`; the why is in that file's docstring). 26.4→4.3s on 4 cores;
the real payoff is the sanitizer builds, which re-run all of it. **A new test
must not share state outside `setUp`'s per-test `mkdtemp` + hashfile** or it
will flake in parallel; `TEST_JOBS=1` is the sequential fallback for pinning
such a flake down. The suite is I/O-bound, so `auto` deliberately
over-subscribes (`2 × nproc`, capped) — don't "fix" it back to `nproc`.
- **A test asserting on the *physical* extent layout must set `serial = True`**
(`DuperemoveTest.serial`), which holds it back to a one-at-a-time pass after
the pool drains. Per-test scratch isolation doesn't help here: the
fsync-forced-extent-boundary trick and fiemap counts depend on btrfs
writeback, which concurrent I/O perturbs — CI caught exactly this on btrfs
(`test_extent_order_independent`, `test_streaming_dedupe`) while xfs passed.
The four `fsync`-boundary files are already marked.

- **Never scan/benchmark out of `/tmp` — it's tmpfs**, not reflink-capable and
rejected by `is_fs_supported()`, so a scan there stores **0 files silently**
and dedupe is a no-op. Use real btrfs/xfs and verify a non-zero file count
Expand Down
9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,14 @@ test:

# End-to-end suite (Python stdlib unittest). Dedupe cases need a reflink fs;
# override the scratch dir with DUPEREMOVE_TEST_DIR=/path.
#
# Worker processes for the suite - tests/run.py's -j, see `tests/run.py --help`.
# Not make's own -j; TEST_JOBS=1 is the sequential fallback. The same value
# sizes the valgrind leg below, where each worker is far heavier.
TEST_JOBS ?= auto
.PHONY: integration
integration: oans
$(SANITIZE_RUN) DUPEREMOVE=./oans python3 tests/run.py
$(SANITIZE_RUN) DUPEREMOVE=./oans python3 tests/run.py -j $(TEST_JOBS)

# Same end-to-end suite, but every oans invocation runs under valgrind memcheck
# (via tests/valgrind-wrap.sh). Findings go to per-pid logs; a non-empty log
Expand All @@ -153,7 +158,7 @@ VGLOGDIR = $(CURDIR)/.vglogs
integration-valgrind: oans
@command -v valgrind >/dev/null 2>&1 || { echo "valgrind not installed"; exit 1; }
rm -rf $(VGLOGDIR) && mkdir -p $(VGLOGDIR)
OANS_VG_LOGDIR=$(VGLOGDIR) DUPEREMOVE=tests/valgrind-wrap.sh python3 tests/run.py
OANS_VG_LOGDIR=$(VGLOGDIR) DUPEREMOVE=tests/valgrind-wrap.sh python3 tests/run.py -j $(TEST_JOBS)
@if find $(VGLOGDIR) -type f -size +0c | grep -q .; then \
echo "=== valgrind reported errors/leaks ==="; \
find $(VGLOGDIR) -type f -size +0c -exec cat {} +; \
Expand Down
12 changes: 12 additions & 0 deletions tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@ Run both with `make check`.

```sh
make integration # build oans and run the suite
make integration TEST_JOBS=1 # ... sequentially (see -j below)
python3 tests/run.py # run directly (binary must be built)
python3 tests/run.py hardlink dedupe # only tests whose id matches a pattern
python3 tests/run.py -j 8 # 8 workers ('auto' by default)
python3 -m unittest discover -s tests/integration -v # plain unittest, no banner
```

The suite runs across worker processes by default, so tests must keep to the
per-test scratch `setUp` hands them. `-j 1` runs them one at a time, which is
where to start if a test only fails in company.

Environment:

- `DUPEREMOVE=/path/to/oans` — test a specific binary (defaults to the one
Expand All @@ -52,6 +58,12 @@ Subclass `DuperemoveTest` (from `harness`) and add `test_*` methods. Each test
gets a fresh scratch directory in `self.work` and a per-test hashfile in
`self.hf`; both are cleaned up automatically.

That isolation is what lets the suite run in parallel, so keep to it — a test
reaching outside its own `self.work` will flake. If a test asserts on the
*physical* extent layout (the fsync-forced extent boundary trick, or fiemap
extent counts), set `serial = True` on the class: no amount of file isolation
helps there, because concurrent I/O changes how the kernel lays extents out.

```python
from harness import DuperemoveTest, requires_reflink

Expand Down
39 changes: 37 additions & 2 deletions tests/integration/harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,41 @@ def scratch_fstype(directory=TEST_ROOT):
# Base test case
# --------------------------------------------------------------------------

_libc = ctypes.CDLL("libc.so.6", use_errno=True)


def _settle_scratch():
"""syncfs() the scratch filesystem so FIEMAP sees real extents.

A test builds files and hands them straight to a scan that asserts on their
extents. Under delayed allocation those extents may not exist yet when oans
maps the file - it correctly records none, and the test reads the shortfall
as a scanner bug. Running the suite in parallel pushes writeback far enough
behind to lose this routinely: test_sparse_file_scans failed 4 runs in 12,
and test_hardlink_pair_does_not_empty_hashfile saw 381 of 401 extents.

Once per oans invocation, not once per file: fsync()ing each created file
costs a journal commit apiece and took the suite from 5.4s to 18s, where one
syncfs() of the whole scratch is a single syscall for the same guarantee.
"""
fd = os.open(TEST_ROOT, os.O_RDONLY)
try:
_libc.syncfs(fd)
finally:
os.close(fd)


class DuperemoveTest(unittest.TestCase):
"""Base class: a fresh scratch dir + hashfile per test, plus helpers."""

# Set True on a class whose assertions depend on the *physical* extent
# layout the kernel happens to produce - the fsync-forced extent boundary
# trick, or fiemap counts. Concurrent I/O perturbs btrfs writeback enough
# that the layout the setup intends is not the one it gets, so tests/run.py
# holds these back and runs them one at a time after the pool drains.
# Per-test scratch isolation is *not* what this is for; that already works.
serial = False

@classmethod
def setUpClass(cls):
if not (os.path.isfile(DUPEREMOVE) and os.access(DUPEREMOVE, os.X_OK)):
Expand All @@ -211,6 +243,7 @@ def dm(self, *args, hashfile=True, stdin=None, env=None, quiet=True):
Runs with -q by default (terse output); pass quiet=False to get the
full human summary block (e.g. to assert on the 'Reclaimed' line).
"""
_settle_scratch() # the tree must be on disk before oans maps it
cmd = [DUPEREMOVE, "--io-threads=4"]
if quiet:
cmd.insert(1, "-q")
Expand Down Expand Up @@ -382,8 +415,10 @@ def make_sparse(self, relpath, head, hole, tail):

def make_trailing_hole(self, relpath, data, size):
"""Write `data`, then extend the file to `size` so it ends in a hole."""
p = self.write(relpath, data)
os.truncate(p, size)
p = self.path(relpath)
with open(p, "wb") as f:
f.write(data)
f.truncate(size)
return p

def reflink(self, src_rel, dst_rel):
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/test_extent_dedupe.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@

@requires_btrfs
class ExtentDedupeTest(DuperemoveTest):
# Extent-layout sensitive: see DuperemoveTest.serial.
serial = True

def _mkfile(self, rel, head, tail):
"""head, an fsync to force an extent boundary, then the shared tail."""
p = self.path(rel)
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/test_extent_order_independent.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

@requires_btrfs
class ExtentOrderIndependentTest(DuperemoveTest):
# Extent-layout sensitive: see DuperemoveTest.serial.
serial = True

def _mkfile(self, rel, head, tail):
"""head, an fsync to force an extent boundary, then the shared tail."""
p = self.path(rel)
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/test_least_fragmented_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

@requires_btrfs
class LeastFragmentedTargetTest(DuperemoveTest):
# Extent-layout sensitive: see DuperemoveTest.serial.
serial = True

def _fragment(self, rel, content):
"""Write content, then rewrite alternate 4K blocks in place (COW) so the
file ends up split across many physical extents."""
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/test_streaming_dedupe.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

@requires_reflink
class StreamingDedupeTest(DuperemoveTest):
# Extent-layout sensitive: see DuperemoveTest.serial.
serial = True

# tiny passes + small batchsize => work spans many generation batches, so
# the producer/watermark and cross-batch anchor paths are all exercised.
ENV = {"DUPEREMOVE_FILES_PER_PASS": "8"}
Expand Down
Loading
Loading