[vavallee]

Summary

Remove torrent after import (#2046, shipped in #2713) had no test that actually ran the removal. This adds them. Test only, no behaviour change, no bug found.

importer (internal/importer/scanner_remove_on_import_test.go), against fake qBittorrent and Transmission daemons:

• qBittorrent, toggle on, for the normal import and both "already in library" shortcuts in checkQbittorrentDownloads: exactly one /torrents/delete for the hash with deleteFiles=false. The fake reads the DB when the delete arrives and sees the row already imported with the file in the library. Default import mode (auto, so hardlink here) leaves the source file in place.
• qBittorrent, toggle off, same three paths: no delete.
• qBittorrent delete answered with HTTP 500, same three paths: row stays imported with no error message, the library file stays, and a cleanup failed warning names the hash and the 500.
• qBittorrent import with no book file: importFailed, no delete.
• Transmission, toggle on: one torrent-remove with ids: [<info hash>] and no delete-local-data (the client only sends that key when it is true), sent after the import committed. Toggle off: none.
• Transmission refusal (HTTP 200, non success result): import kept, reason logged.
• Transmission legacy numeric id row: the import lands, RemoveTransmissionTorrent refuses the number, the daemon never sees a torrent-remove, and the refusal is logged.
• Transmission import with no book file: importFailed, no removal.

db (internal/db/download_clients_remove_on_import_test.go):

• Round trip of RemoveOnImport true and false through Create, Update (flipped both ways), GetByID, List, ListEnabled, GetEnabledByProtocol, GetFirstEnabled and GetFirstEnabledByProtocol.
• Pre 090 row: drops the column, inserts a client, reruns migration 090 and checks it reads false everywhere.
• A non integer remove_on_import is a scan error in every read path, not a silent false.

Fail before checks, run locally and reverted: flipping deleteFiles to true in both cleanups fails 4 subtests, and moving the shortcut cleanup ahead of the imported write fails both shortcut subtests.

No changelog fragment: nothing in .github/workflows checks for one, and these are tests only.

Covered lines

go test ./internal/importer/ ./internal/db/ -coverprofile, before is origin/main at d34e906. The db baseline was taken without the new db test file. Counts are set mode, so 0 means never run and 1 means run.

block	before	after
scanner_poll.go:626.91,627.39 (shortcut 1 cleanup call)	0	1
scanner_poll.go:627.39,629.8 (shortcut 1 cleanup failed warn)	0	1
scanner_poll.go:684.91,685.39 (shortcut 2 cleanup call)	0	1
scanner_poll.go:685.39,687.8 (shortcut 2 cleanup failed warn)	0	1
scanner_poll.go:1021.92,1023.26 (Transmission cleanup built)	0	1
scanner_poll.go:1023.26,1027.87 (RemoveTransmissionTorrent call)	0	1
scanner_poll.go:1027.87,1029.5 (removal error wrapped)	0	1
scanner_poll.go:1030.4,1030.14 (removal ok)	0	1
scanner_poll.go:1244.2,1245.22 (qBittorrent cleanup closure)	0	1
scanner_poll.go:1245.22,1247.60 (qb.DeleteTorrent call)	0	1
scanner_poll.go:1247.60,1249.4 (delete error wrapped)	0	1
scanner_poll.go:1250.3,1250.13 (delete ok)	0	1
download_clients.go:148.71,150.4 (List scan error)	0	1
download_clients.go:183.96 to 208.28 (ListEnabled, all blocks except the query error)	0	1
download_clients.go:279.71,281.4 (GetEnabledByProtocol scan error)	0	1

Package totals: importer 85.0% to 85.3%, db 72.0% to 72.4%.

Test plan

• go build ./...
• go vet ./...
• go test ./internal/importer/... ./internal/db/... ./internal/downloader/... -count=1
• go test -race ./internal/importer/ ./internal/db/ -run RemoveOnImport -count=1
• Coverage before and after, table above
• Mutation checks described above fail the new tests and were reverted
• golangci-lint (not run locally)

🤖 Generated with Claude Code

https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

[github-actions]

Test-only, no behaviour change — looks solid overall.

One gap vs. the PR description: TestDownloadClientRepo_UnreadableRemoveOnImportIsAnError tests four read paths (List, ListEnabled, GetEnabledByProtocol, GetByID) but skips GetFirstEnabled and GetFirstEnabledByProtocol. Both scan remove_on_import as int (download_clients.go:219, download_clients.go:245), so a text value in that column would error there too. The PR description says "a scan error in every read path, not a silent false" — the claim slightly overstates coverage. Two extra assertions at the bottom of that test would close it.

Everything else checks out: mutexes on both fake daemons, t.Cleanup restoring the global logger, migration 090_download_client_remove_on_import.sql confirmed present, all referenced helpers and production functions exist, and the legacy-numeric-id guard matches RemoveTransmissionTorrent's strconv.ParseInt check at adapter.go:356.

— 🤖 Bindery triage bot (automated). Reply to correct me; a human will see it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!