test: make three vacuous tests actually test something; drop one duplicate
The test suite is not bloated -- 3,701 lines of test code against 3,760 lines of product code, one duplicate pair in 356 tests, 8% single-assertion tests. The waste was not volume, it was four tests that looked like coverage and provided none: - test_parents_are_mounted_before_children: the fixture was already in depth order, so the sort it exists for was never exercised. Deleting `mounts.sort(key=_depth)` passed the whole suite. It now uses datasets whose NAME order differs from their MOUNTPOINT depth, which is the only case the sort is for. - test_it_NEVER_touches_the_current_run: the "current" snapshot was a minute old, so the age floor excluded it regardless and the same-snapname guard never ran. That guard only matters for a run that OUTLIVES the floor -- which a first full upload easily does, and where collecting it would yank the snapshot out from under a backup that is still reading from it. Now tested with a 12-hour-old current run. - the fingerprint/unknown test put the unreadable file in `providers`, which is not broken -- so fingerprint() skipped the whole module via is_broken() and the filter under test never executed. The blip has to land in the module that IS broken. - test_the_real_truenas_versions was a byte-identical copy of test_async_middleware_is_detected under a name promising more. Replaced with the fact actually worth pinning: the flavour probe must read STOCK source, because apply.sh re-runs on an already-patched overlay and our own SNAPSHOT_SYNC block is a plain `def create_snapshot` -- reading it would report a 25.10 box as synchronous and inject the wrong wrapper. All four now fail when the code they name is broken.
This commit is contained in:
+23
-4
@@ -304,9 +304,23 @@ class TestAsyncFlavour:
|
||||
broken["plugins/cloud_backup/sync.py"] = Unreadable("HTTP 429")
|
||||
assert compat.async_flavour(loader(broken)) is None
|
||||
|
||||
def test_the_real_truenas_versions(self):
|
||||
# Pinning the actual fact this whole port exists for.
|
||||
assert compat.async_flavour(loader(GOOD)) is True
|
||||
def test_it_reads_STOCK_source_not_our_own_injected_block(self):
|
||||
# This was a byte-identical copy of test_async_middleware_is_detected under a
|
||||
# name that promised more. The fact worth pinning: apply.sh re-runs on an
|
||||
# ALREADY-PATCHED overlay, so the probe must cut our block off first -- our own
|
||||
# SNAPSHOT_SYNC wrapper is a plain `def create_snapshot`, and reading it would
|
||||
# report a 25.10 box as synchronous and inject the wrong flavour.
|
||||
patched = dict(GOOD)
|
||||
patched["plugins/cloud/snapshot.py"] = (
|
||||
GOOD["plugins/cloud/snapshot.py"]
|
||||
+ "\n# TRUECLOUD_PATCH\n"
|
||||
+ 'def create_snapshot(middleware, path, name="x"):\n return "s", "p"\n'
|
||||
)
|
||||
assert compat.async_flavour(loader(patched)) is True, (
|
||||
"the flavour probe read our own injected block and concluded the box is "
|
||||
"synchronous -- it would then inject a sync wrapper into an async "
|
||||
"middleware, and every nested backup would break"
|
||||
)
|
||||
|
||||
|
||||
class TestMiddlewareMethodsWeCall:
|
||||
@@ -530,6 +544,11 @@ class TestATransientNetworkBlipDoesNotWakeAnybody:
|
||||
return [{"ref": "master", "modules": check_files(files)}]
|
||||
|
||||
def test_an_unreadable_file_does_not_change_the_fingerprint_of_a_broken_ref(self):
|
||||
# The blip must land in the SAME module that is broken. Put it in `providers`
|
||||
# (which is healthy) and `fingerprint()` skips the whole module via
|
||||
# `is_broken(m)` -- so the `state` filter under test never runs and the test
|
||||
# passes no matter what the code does. `nested` is the broken one here, so the
|
||||
# unreadable file goes in `nested` too.
|
||||
broken = with_(**{
|
||||
"plugins/cloud/snapshot.py":
|
||||
"async def create_snapshot(name, path, middleware):\n return 1, 2\n",
|
||||
@@ -537,7 +556,7 @@ class TestATransientNetworkBlipDoesNotWakeAnybody:
|
||||
clean = compat.fingerprint(self._rows(broken))
|
||||
|
||||
blipped = dict(broken)
|
||||
blipped["rclone/remote/b2.py"] = Unreadable("HTTP 429")
|
||||
blipped["plugins/cloud_backup/sync.py"] = Unreadable("HTTP 429") # nested
|
||||
assert compat.fingerprint(self._rows(blipped)) == clean, (
|
||||
"a rate-limited fetch changed the fingerprint, so the bot rewrites the "
|
||||
"issue body and then rewrites it back tomorrow"
|
||||
|
||||
@@ -143,13 +143,28 @@ class TestPlanStaging:
|
||||
)
|
||||
|
||||
def test_parents_are_mounted_before_children(self):
|
||||
# A child's mountpoint dir only exists inside its parent's snapshot, so
|
||||
# mounting a child first would fail.
|
||||
mounts, _ = plan()
|
||||
# A child's mountpoint dir only exists inside its PARENT's snapshot, so
|
||||
# mounting a child first fails.
|
||||
#
|
||||
# The default fixture is already in depth order, so it never exercised the
|
||||
# sort at all -- deleting `mounts.sort(key=_depth)` passed the whole suite.
|
||||
# These datasets are deliberately in the WRONG order by name: `Tap/aaa` is
|
||||
# three levels deep and `Tap/zzz` is one, so anything that preserves input
|
||||
# order (or sorts by name) mounts the child first and would fail for real.
|
||||
awkward = [
|
||||
ds("Tap", "/mnt/Tap"),
|
||||
ds("Tap/aaa", "/mnt/Tap/zzz/deep/deeper"),
|
||||
ds("Tap/zzz", "/mnt/Tap/zzz"),
|
||||
ds("Tap/mmm", "/mnt/Tap/zzz/deep"),
|
||||
]
|
||||
mounts, _ = plan(datasets=awkward)
|
||||
|
||||
seen = set()
|
||||
for _src, target in mounts:
|
||||
if target != ROOT:
|
||||
assert os.path.dirname(target) in seen
|
||||
assert os.path.dirname(target) in seen, (
|
||||
f"{target} is mounted before its parent exists"
|
||||
)
|
||||
seen.add(target)
|
||||
|
||||
def test_backup_path_below_dataset_root(self):
|
||||
@@ -1088,6 +1103,27 @@ class TestGarbageCollectorSelection:
|
||||
names = [self.CURRENT, "Tap/apps/x@cloud_backup-5-20260714115900"]
|
||||
assert self.collect(names) == []
|
||||
|
||||
def test_it_NEVER_touches_the_current_run_EVEN_WHEN_IT_IS_OLD(self):
|
||||
# The one that matters, and the one that was not tested: a backup running
|
||||
# longer than the age floor. The old test's `current` was a minute old, so the
|
||||
# floor excluded it anyway and the same-snapname guard never ran -- deleting
|
||||
# that guard passed the whole suite.
|
||||
#
|
||||
# A first full upload of a 100 GB pool takes hours. If the GC collected the
|
||||
# snapshot of the run that is CURRENTLY READING FROM IT, restic would be
|
||||
# yanked out from under itself mid-backup.
|
||||
import datetime as dt
|
||||
|
||||
old_current = "Tap@cloud_backup-5-20260714000000" # 12 hours old
|
||||
names = [old_current, "Tap/apps/x@cloud_backup-5-20260714000000"]
|
||||
assert tn.stale_snapshot_names(
|
||||
"cloud_backup-5", old_current, names,
|
||||
dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC),
|
||||
) == [], (
|
||||
"the GC collected the snapshot of the run that is using it. A long first "
|
||||
"upload would be destroyed mid-flight."
|
||||
)
|
||||
|
||||
def test_it_NEVER_touches_a_periodic_snapshot(self):
|
||||
assert self.collect(["Tap/apps/x@auto-2026-07-13_03-00"]) == []
|
||||
|
||||
|
||||
Reference in New Issue
Block a user