Skip to content

Commit f8b5054

Browse files
committed
test(memory): assert queue_depth delta to survive sibling spawn_manual_sync pollution
`memory_ingestion_status_reflects_initialized_client_snapshot` was asserting `status.queue_depth == 1` against the process-global `IngestionState`, on the assumption that the lock acquired at the top of the test guarantees a clean baseline. The two sibling tests (`memory_sync_channel_publishes_targeted_event` and `memory_sync_all_publishes_broadcast_event`) call `memory_sync_channel` / `memory_sync_all` → `spawn_manual_sync`, which detaches a `tokio::spawn(...)` background task that runs `composio::run_connection_sync`. That detached task can enqueue an ingestion job via `MemoryClient::store_skill_sync` → `IngestionQueue::submit` → `state.enqueue()`. By the time this test acquires `GLOBAL_MEMORY_TEST_LOCK`, that background work may already have bumped the global counter — so the equality assertion fires with `left: 2, right: 1`. CI hit this on #2720 (\"Rust Core Tests + Quality\") despite the lock because the lock only serialises against tests that *also* acquire it — not the detached worker that was spawned earlier and is still draining. The same flake is reachable on `upstream/main` in CI with a parallel-test ordering that puts the snapshot test directly after one of the sync-channel tests; my PR's added vault sync tests didn't cause it, but they shifted the ordering enough to expose it. The principled fix is to snapshot the baseline `queue_depth` at the start of the test and assert the delta. That keeps the test's contract intact (an `enqueue + mark_running` round-trip must surface in the snapshot) without depending on the rest of the suite leaving the global state pristine. Targeted run on the failure window passes: cargo test --lib -- memory::ops::sync vault::sync → 9/9 pass
1 parent b8bdb09 commit f8b5054

1 file changed

Lines changed: 10 additions & 1 deletion

File tree

src/openhuman/memory/ops/sync.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,15 @@ mod tests {
367367
let client = ensure_memory_client();
368368
let state = client.ingestion_state();
369369

370+
// The other `memory::ops::sync` tests (`memory_sync_channel_*`,
371+
// `memory_sync_all_*`) call `spawn_manual_sync`, which detaches a
372+
// `tokio::spawn` background task that may still be in flight — and
373+
// may have enqueued an ingestion job — by the time this test
374+
// acquires `GLOBAL_MEMORY_TEST_LOCK`. The global `IngestionState`
375+
// singleton therefore can't be assumed to start at queue_depth=0.
376+
// Capture the baseline and assert the delta instead.
377+
let baseline_depth = state.snapshot().queue_depth;
378+
370379
state.enqueue();
371380
state.mark_running("doc-sync", "Sync Title", "sync-test");
372381

@@ -379,7 +388,7 @@ mod tests {
379388
assert_eq!(status.current_document_id.as_deref(), Some("doc-sync"));
380389
assert_eq!(status.current_title.as_deref(), Some("Sync Title"));
381390
assert_eq!(status.current_namespace.as_deref(), Some("sync-test"));
382-
assert_eq!(status.queue_depth, 1);
391+
assert_eq!(status.queue_depth, baseline_depth + 1);
383392

384393
state.dequeue();
385394
state.mark_completed("doc-sync", true, 12345);

0 commit comments

Comments
 (0)