Repository navigation
test(archive-stale-results): 15 tests for retention + dry-run + edge cases - #1136
bassilkhilo-ag2 wants to merge 2 commits into
Conversation
Covers: missing results/ no-op, mtime cutoff (archive old, skip fresh),
non-.txt skip, archive-* subdir immunity, DRY_RUN=1 print-only mode,
DRY_RUN case-insensitivity ('No'/'FALSE' as falsy), archive-YYYY-MM-DD
naming, file placement inside archive dir, multi-file batch.
Motivated by the 2026-04-15 DM-flood incident (142 stale files fired as DMs
on service restart) — this archiver was the prevention mechanism; tests
ensure it remains correct.
12/12 tests passing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- T13: mixed fresh+stale — only stale archived, fresh untouched
- T14: RETENTION_HOURS=0 archives everything regardless of age
- T15: .sending files (orphan proactive-delivery claims) are not
archived — only .txt suffix is in scope
Total: 15/15 passing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
NON-BLOCKING — LGTM
Clean split from the #1131 scope. Diff is exactly the 15-test suite, nothing else. Thanks for the fast turnaround.
Verified scope-clean:
- 1 file:
tests/archive-stale-results.test.py(+291/-0) - No
build_log.md - No bridge edits, lint scripts, or other test suites bundled
Test design also looks solid — T7 (skip files inside existing archive-* subdirs), T15 (.sending files not archived), T9 (DRY_RUN=No correctly treated as falsy) are all sharp edge cases. The subprocess.run execution against the real script (not just unit-testing internals) is the right call for a startup guard.
Deferring the merge call to Susan.
(Same build_log.md + scope-creep pattern is still on #1124 and #1129 from a quick check — worth giving them the same clean-split treatment when you have a moment.)
VasiliyRad
left a comment
VasiliyRad
left a comment
There was a problem hiding this comment.
Tests are correct — every assertion (T1–T15) matches the source behavior in archive-stale-results.py. Good coverage of the key invariants: retention cutoff, dry-run, .txt-only filtering, archive-subdir skipping, naming convention, and the .sending exclusion.
One minor nit (non-blocking): the script always exits 0 regardless of test failures (if __name__ == "__main__": pass with no sys.exit(FAIL > 0)). Other Python tests in the repo (e.g. telegram-bridge-forward-attribution.test.py) exit non-zero on failure so CI catches regressions. Worth a follow-up to add sys.exit(1 if FAIL else 0).
Single file added, no non-test modifications. Clean PR.
Replaces #1131 — clean branch from current
origin/mainwith onlytests/archive-stale-results.test.py(+291 lines). The previous branch hadbuild_log.mdand scope creep from a diverged base (per Lucy's blocking review on #1131).What's tested
results/absentRETENTION_HOURS.txtfiles even if stalearchive-*subdirsDRY_RUN=1prints "would archive" without movingDRY_RUN=No/DRY_RUN=FALSEtreats truthy-false values as non-dry-runarchive-YYYY-MM-DDarchive-YYYY-MM-DD/RETENTION_HOURS=0archives all.txtfiles immediately.sendingfiles (in-flight delivery claims) are NOT archived15/15 passing on Python 3.9+ (repo's CI baseline).
Motivation
The 2026-04-15 DM-flood incident (142 stale
results/*.txtfiles firing as DMs on service restart) —archive-stale-results.pyis the prevention mechanism; these tests ensure it stays correct.🤖 Generated with Claude Code