test(import): undo monkeypatches before cleanup deletes buckets

[ErikBjare]

Fixes the bundle's integration tests, which fail on every Qt build since aw-server was bumped to 1780f2a (ActivityWatch/activitywatch@bc96b3d): tests/test_server.py::test_buckets gets assert 4 == 1.

Cause: fixtures tear down in reverse setup order. test_import_rollback_failure_is_reported_as_server_error and test_import_rollback_failure_does_not_abort_other_deletes (from #176) list monkeypatch before cleanup, so cleanup runs while app.api.delete_bucket is still patched to fail. The deletes fail silently and leak test-import-rb-a, -rb-b and -rb-c into the session-scoped testing DB. The bundle runs test_import.py before test_server.py, which then counts 4 buckets. This repo's CI only runs an import check, so nothing caught it here.

Fix: cleanup takes monkeypatch and calls monkeypatch.undo() before deleting, so it works whatever the argument order. undo() is idempotent, so monkeypatch's own teardown is unaffected.

Verified (scratch HOME):

• pytest tests/test_import.py tests/test_server.py: 1 failed → 31 passed.
• A leak-detecting autouse fixture reports no leaked buckets after the change (before: the 3 above).
• Full tests/ without a running server: the only difference is test_buckets now passing; the other 16 need a live server (Connection refused) and fail the same with or without this change.

[ErikBjare]

@greptileai review
@TimeToBuildBob review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T14:32:36.062000Z	1d891cc	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge; cleanup restores the working delete method before removing test buckets.

Summary

Updates cleanup to request monkeypatch and undo patches before deleting test buckets.

• Import tests restore patched deletes before cleaning up buckets.

_{Reviews (1) · Last reviewed commit: "test(import): undo monkeypatches before ..." · Reviewed by Greptile}

[TimeToBuildBob]

Reviewing this now: I’ll check the fixture dependency/teardown ordering and rerun the import → server test sequence in an isolated HOME, including a base-versus-head comparison.

[TimeToBuildBob]

Reviewed 1d891cc07052d22b11bda000cb8b729049f2c2cf: no blocking findings.

Independently reproduced the exact import → server ordering in separate scratch HOME/XDG directories (Python 3.11; benchmarks disabled):

• Parent commit: 30 passed, 1 failed, with test_buckets reporting assert 4 == 1 and the three leaked rollback buckets (test-import-rb-a, -rb-b, -rb-c).
• PR head: 31 passed.

The explicit cleanup → monkeypatch dependency keeps the fixture available through cleanup, and monkeypatch.undo() restores the real delete method before cleanup issues DELETE requests. The rollback assertions still run against the patched method during the test; monkeypatch's subsequent teardown safely has nothing left to undo. This is a scoped test-fixture fix, with no production behavior change.

GitHub CI was queued at the initial check; these results verify the affected local sequence, not the full live-server suite or the platform matrix. Recommend merging once required CI passes.

[TimeToBuildBob]

🤖 AI code review

Modifies the cleanup fixture in tests/test_import.py to accept pytest's monkeypatch fixture and call monkeypatch.undo() during teardown before fetching and deleting buckets. This prevents tests that patch app.api.delete_bucket with a failing implementation from leaking buckets into the session-scoped testing DB, since cleanup tears down before monkeypatch's own teardown.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings. The diff looks correct to me on this pass.

Files changed (1) — the diff as I read it

• tests/test_import.py — Adds a monkeypatch parameter to the cleanup fixture and calls monkeypatch.undo() before deleting buckets in teardown.

_{Reviewed 1d891cc07052 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 348s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

Reviewed. Fix is correct.

The teardown order issue: pytest tears down fixtures in reverse dependency order, so a test that lists monkeypatch before cleanup has cleanup run first — meaning app.api.delete_bucket might still be patched to fail when cleanup tries to delete buckets.

monkeypatch.undo() before the deletion loop ensures the real delete_bucket is always called in cleanup, preventing bucket leaks into later tests.

5/5 AI review + macOS/Windows/format/lint all green. LGTM.