diff --git a/FOLLOW_UP_2.md b/FOLLOW_UP_2.md index 8ed0068..e853d51 100644 --- a/FOLLOW_UP_2.md +++ b/FOLLOW_UP_2.md @@ -17,48 +17,31 @@ The OperationGuard ordering fix in `handlers.rs` is the structurally most import replaces its UI operation map from that snapshot, and the React game-list merge uses the snapshot to clear stale download/install/update/uninstall spinners even when a begin/finish/fail event was missed. +- Install-recovery matrix: `recovery_covers_install_matrix_rows` now covers the + ten named Installing / Updating / Uninstalling rows from PLAN.md, including + markerless reserved directories while an intent proves ownership. +- Update rollback on commit-rename failure: + `update_commit_rename_failure_restores_previous_local` forces a post-extract + commit conflict without production hooks and verifies backup restore. +- Uninstall delete-failure restore: + `uninstall_delete_failure_restores_backup` covers the rollback path on Unix, + where directory permissions can force recursive delete failure without hooks. +- Installed-only → Ready rescan: + `rescan_promotes_installed_only_game_to_ready_when_sentinel_appears` pins the + cached-index transition after a user adds `version.ini` to a local-only root. +- Scanner gating dispatch: local monitor tests now cover active-operation event + drops, burst collapse, fallback sideload pickup, and non-catalog roots leaving + no library entry or delta. +- Serve gating dispatch: stream tests now cover `GetGame` response gates and + the shared full-file/chunk transfer gate for non-catalog, active-operation, + missing-sentinel, and local-path cases. +- TempDir test helper: peer tests now share `test_support::TempDir` instead of + carrying per-module copies. ## Still open -### Tests still missing - -#### 2. Install-recovery matrix (~10 rows) - -Only two intent-driven rows are covered: `Updating | no local | .local.installing | .local.backup` (`recovery_restores_backup_for_interrupted_update`) and the active-skip case (`startup_recovery_skips_active_game_roots`). The other ~9 combinations of `(intent_state ∈ {Installing, Updating, Uninstalling}, local present?, .local.installing present?, .local.backup present?)` are unverified. - -**Approach:** convert `recovery_restores_backup_for_interrupted_update` into a table-driven test. Iterate over the named matrix rows in PLAN.md, set up FS + intent, run `recover_game_root`, assert resulting FS + intent. - -#### 3. Update rollback on commit-rename failure - -Only extract-failure is tested (`update_failure_restores_previous_local`). The branch where extract succeeds but the staging→local rename fails is unexercised. Force it by making `local/` un-renamable (e.g. swap in a directory under that name post-extract) or by injecting a rename hook. - -#### 4. Uninstall delete-failure restore - -`restore_backup` rollback in `transaction.rs:107-114` is unreached by tests. - -#### 5. Installed-only → Ready rescan - -PLAN listed this. No test exercises the transition where a user drops `version.ini` into a `local/`-only game root and the next rescan flips it to Ready. - -#### 6. Scanner gating dispatch - -`handle_watch_event` and `RescanGate` / `run_gated_rescan` (`local_monitor.rs:208-287`) have zero tests. Cover: - -- Event for ID under active operation is dropped. -- Burst of events collapses to ≤2 rescans for the same ID. -- Sideload picked up by fallback scan. -- Non-catalog game produces no library index entry and no `LibraryDelta`. - -#### 7. Serve gating dispatch - -`local_download_available` predicate is tested; `handle_get_game_command`, `handle_get_game_file_data`, `handle_get_game_file_chunk` dispatch paths aren't. Small tests against an in-memory `Ctx` covering: non-catalog ID, mid-operation, missing sentinel. - ### Code hygiene -#### 8. Consolidate `TempDir` test helper (getting worse) - -Reimplemented in **five** test modules now: `install/intent.rs`, `install/transaction.rs`, `local_games.rs`, `download.rs`, `handlers.rs`. Each round of new tests adds another copy. Move to a single `test_support` module (or use the `tempfile` crate, already stable). The longer this waits, the bigger the eventual consolidation diff. - #### 9. `availability` field is `String`, not a typed enum `lanspread-db::Game::availability: String` with `AVAILABILITY_*` consts works but allows invalid values. `remote_peer::summary_from_game` already has a defensive fallback for unknown strings. A typed enum + serde would be tighter, but is a wire-format change — coordinate if/when other peers are upgraded.