Skip to content

Commit c0aed65

Browse files
committed
Add state-transition tests for IndexPhase
Cover the Initializing -> Disabled transition through stop_indexing's public path (the race that drops the half-built manager when the user disables indexing while resume_or_scan is still walking), plus the two no-op arms (stop from Disabled, clear from Initializing) that protect the in-flight start path from being clobbered. The start_indexing path (Disabled -> Initializing -> Running) needs an AppHandle and a real IndexManager, which can't be stood up in a unit test. Extracted the post-resume_or_scan classifier (is_initializing_phase) as a pure helper so the decision the race relies on ("are we still the same Initializing entry we started as?") is testable in isolation. Tests serialize on a dedicated mutex and always restore INDEXING to Disabled, so the global cell stays in a known-good state between cases.
1 parent 9a9899e commit c0aed65

1 file changed

Lines changed: 122 additions & 7 deletions

File tree

  • apps/desktop/src-tauri/src/indexing

apps/desktop/src-tauri/src/indexing/mod.rs

Lines changed: 122 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,18 @@ pub fn stop_indexing() -> Result<(), String> {
167167
Ok(())
168168
}
169169

170+
/// Phase classifier used by `start_indexing`'s post-`resume_or_scan` branch.
171+
/// Returns true only while the phase carries the temporary init store. If
172+
/// `stop_indexing` swapped the state out from under us during `resume_or_scan`,
173+
/// the phase is `Disabled` (or briefly `ShuttingDown`) and this returns false
174+
/// — the caller treats that as "phase changed, shut the manager down".
175+
///
176+
/// Extracted as a pure helper so the state-machine race fragment is testable
177+
/// without standing up an `AppHandle` / `IndexManager`.
178+
pub(crate) fn is_initializing_phase(phase: &IndexPhase) -> bool {
179+
matches!(phase, IndexPhase::Initializing { .. })
180+
}
181+
170182
/// Create the IndexManager for the root volume and auto-start indexing
171183
/// (resume from existing index or fresh scan).
172184
///
@@ -200,11 +212,11 @@ pub fn start_indexing(app: &AppHandle) -> Result<(), String> {
200212
let writer_for_vacuum = manager.writer.clone();
201213

202214
// Re-lock and check: if someone called stop_indexing() while we were
203-
// inside resume_or_scan(), the phase is now Disabled. Respect that —
204-
// shut down the manager instead of overwriting with Running.
215+
// inside resume_or_scan(), the phase is no longer Initializing.
216+
// Respect that — shut down the manager instead of overwriting.
205217
let mut guard = INDEXING.lock().map_err(|e| format!("Failed to lock state: {e}"))?;
206-
match (&*guard, scan_result) {
207-
(IndexPhase::Initializing { .. }, Ok(())) => {
218+
match (is_initializing_phase(&guard), scan_result) {
219+
(true, Ok(())) => {
208220
*guard = IndexPhase::Running(Box::new(manager));
209221
log::info!("start_indexing: done — IndexManager is Running");
210222

@@ -219,19 +231,19 @@ pub fn start_indexing(app: &AppHandle) -> Result<(), String> {
219231
}
220232
});
221233
}
222-
(IndexPhase::Initializing { .. }, Err(e)) => {
234+
(true, Err(e)) => {
223235
*guard = IndexPhase::Disabled;
224236
if let Some(pool) = READ_POOL.lock().unwrap().take() {
225237
pool.invalidate();
226238
}
227239
return Err(e);
228240
}
229-
(_, Ok(())) => {
241+
(false, Ok(())) => {
230242
// Phase changed (e.g. stop_indexing set Disabled). Don't override.
231243
log::info!("start_indexing: phase changed during init, shutting down manager");
232244
manager.shutdown();
233245
}
234-
(_, Err(e)) => {
246+
(false, Err(e)) => {
235247
log::warn!("start_indexing: resume_or_scan failed and phase changed: {e}");
236248
manager.shutdown();
237249
}
@@ -1169,6 +1181,109 @@ mod tests {
11691181
));
11701182
}
11711183

1184+
// ── IndexPhase transitions ─────────────────────────────────────────
1185+
//
1186+
// The global INDEXING cell is shared with the running app (and with the
1187+
// verifier::trigger_verification path), so these tests serialize via a
1188+
// dedicated mutex and always restore the cell to Disabled before
1189+
// returning. They never call `start_indexing` (needs an AppHandle) —
1190+
// instead they install an `Initializing { store }` phase by hand and
1191+
// drive the transitions whose Rust-side state machine is reachable
1192+
// without a Tauri runtime: stop_indexing's Initializing -> Disabled
1193+
// arm, and clear_index's no-op arm when not Running.
1194+
1195+
static INDEXING_TEST_GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(());
1196+
1197+
/// Replace INDEXING with `Disabled` and clear READ_POOL. Used at the
1198+
/// start of each IndexPhase test so transient state from earlier tests
1199+
/// (or the running app, if these tests are run inside a debug build
1200+
/// with the app warmed up) doesn't bleed in.
1201+
fn reset_indexing_for_test() {
1202+
let mut guard = INDEXING.lock().expect("INDEXING lock poisoned");
1203+
*guard = IndexPhase::Disabled;
1204+
drop(guard);
1205+
// The stop/clear paths invalidate READ_POOL; mirror that so we
1206+
// don't carry a stale pool from a prior test.
1207+
*READ_POOL.lock().unwrap() = None;
1208+
}
1209+
1210+
fn install_initializing_phase() -> tempfile::TempDir {
1211+
let dir = tempfile::tempdir().expect("temp dir for init store");
1212+
let db_path = dir.path().join("init-phase-test.db");
1213+
let store = store::IndexStore::open(&db_path).expect("open init store");
1214+
let mut guard = INDEXING.lock().expect("INDEXING lock poisoned");
1215+
*guard = IndexPhase::Initializing { store };
1216+
dir
1217+
}
1218+
1219+
#[test]
1220+
fn is_initializing_phase_matches_only_initializing_variant() {
1221+
let dir = tempfile::tempdir().expect("temp dir");
1222+
let store =
1223+
store::IndexStore::open(&dir.path().join("classifier.db")).expect("open store");
1224+
// Disabled / ShuttingDown / Running classified as not-initializing.
1225+
assert!(!is_initializing_phase(&IndexPhase::Disabled));
1226+
assert!(!is_initializing_phase(&IndexPhase::ShuttingDown));
1227+
// Initializing classified as initializing.
1228+
assert!(is_initializing_phase(&IndexPhase::Initializing { store }));
1229+
}
1230+
1231+
#[test]
1232+
fn stop_indexing_during_initialization_transitions_to_disabled() {
1233+
// Pins the Initializing -> Disabled race arm in stop_indexing
1234+
// (mod.rs:158). If `stop_indexing` runs while `start_indexing` is
1235+
// inside `resume_or_scan`, the phase must be cleared to Disabled
1236+
// so the post-scan re-lock observes the change and shuts the
1237+
// half-built manager down.
1238+
let _guard = INDEXING_TEST_GUARD.lock().unwrap_or_else(|e| e.into_inner());
1239+
reset_indexing_for_test();
1240+
1241+
let _tmp = install_initializing_phase();
1242+
stop_indexing().expect("stop_indexing must succeed from Initializing");
1243+
1244+
let phase_guard = INDEXING.lock().expect("INDEXING lock poisoned");
1245+
assert!(
1246+
matches!(*phase_guard, IndexPhase::Disabled),
1247+
"stop_indexing must collapse Initializing to Disabled"
1248+
);
1249+
drop(phase_guard);
1250+
reset_indexing_for_test();
1251+
}
1252+
1253+
#[test]
1254+
fn stop_indexing_when_disabled_is_a_noop() {
1255+
// Pins the catch-all arm in stop_indexing (mod.rs:162): if the
1256+
// phase isn't Running or Initializing, the original phase must be
1257+
// restored (not silently replaced with Disabled).
1258+
let _guard = INDEXING_TEST_GUARD.lock().unwrap_or_else(|e| e.into_inner());
1259+
reset_indexing_for_test();
1260+
1261+
// Already Disabled; stop_indexing should remain Disabled (no-op).
1262+
stop_indexing().expect("stop_indexing from Disabled must succeed");
1263+
let phase_guard = INDEXING.lock().expect("INDEXING lock poisoned");
1264+
assert!(matches!(*phase_guard, IndexPhase::Disabled));
1265+
drop(phase_guard);
1266+
}
1267+
1268+
#[test]
1269+
fn clear_index_when_not_running_is_a_noop() {
1270+
// Pins the catch-all arm in clear_index (mod.rs:274): clear must
1271+
// not touch the DB or change phase unless Running. Initializing
1272+
// is preserved so an in-flight start_indexing can keep walking.
1273+
let _guard = INDEXING_TEST_GUARD.lock().unwrap_or_else(|e| e.into_inner());
1274+
reset_indexing_for_test();
1275+
1276+
let _tmp = install_initializing_phase();
1277+
clear_index().expect("clear_index from Initializing must succeed");
1278+
let phase_guard = INDEXING.lock().expect("INDEXING lock poisoned");
1279+
assert!(
1280+
matches!(*phase_guard, IndexPhase::Initializing { .. }),
1281+
"clear_index must preserve a non-Running phase"
1282+
);
1283+
drop(phase_guard);
1284+
reset_indexing_for_test();
1285+
}
1286+
11721287
/// After clearing READ_POOL, `enrich_entries_with_index` returns early
11731288
/// without panic and leaves entries unenriched.
11741289
#[test]

0 commit comments

Comments
 (0)