test: extract shared assert_pending_close_task_runs_to_completion helper

[dhruvbatra]

Issue

test_mcp_server.py::test_close_runners_sync_holds_strong_reference_until_task_completes and test_navigator_client.py::test_schedule_close_holds_strong_reference_until_task_completes each end with a byte-identical 9-line block asserting the same "the task must be retained in _pending_close_tasks until it finishes, then released" regression-guard invariant — differing only in which module (mcp_server vs navigator_client) they check.

Found via a fresh AST sliding-window duplicate scan across all tests/*.py (same technique that found PRs #307 and #308 earlier today), which flagged this as the only cross-file duplicate — most other matches were same-file or too small/coincidental to be worth extracting.

Fix

Hoisted the block to a single assert_pending_close_task_runs_to_completion(module, release, closed) async helper in tests/fakes.py (the file both test modules already treat as shared scaffolding), and had both tests call it. This matches the file's established convention of factoring out duplicated test scaffolding (assert_claim_result_payload_shape, noop_sleep, import_or_skip, etc.) that PRs #307/#308 also followed today.

Why it's safe

• Test-only change; no production code touched.
• Pure extraction — the helper body is a verbatim copy of the duplicated code, no logic altered.
• uv run pytest tests/ before and after: 326 passed, same 21 pre-existing environment-only Chromium-sandbox failures (verified unchanged by diffing full test output pre/post-change).
• uv run ruff check src/ tests/ clean; uv run ruff format --check on the 3 changed files reports "already formatted" (the 7 files flagged by a full-tree format-check are pre-existing and unrelated — reproduced identically on a git stash of this change).

Co-authored-by: Claude noreply@anthropic.com

---

Generated by Claude Code

---

Note

Low Risk
Test-only refactor with verbatim logic moved to a shared helper; no production code changes.

Overview
Deduplicates identical regression-guard tails in the MCP server and navigator client tests by moving them into assert_pending_close_task_runs_to_completion in tests/fakes.py.

That helper still drives the same async close scenario: yield so the background task blocks on an event, assert the module keeps exactly one entry in _pending_close_tasks, release the event, await the retained task, then assert cleanup ran and the pending set is empty. test_close_runners_sync_holds_strong_reference_until_task_completes and test_schedule_close_holds_strong_reference_until_task_completes now call it instead of inlining the block; fakes.py adds the asyncio import the helper needs.

^{Reviewed by Cursor Bugbot for commit 14513ef. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Tests
  • Consolidated repeated asynchronous close-task verification into a shared test helper.
  • Preserved regression coverage ensuring pending close tasks complete and clean up correctly in server and client scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b5ad1f68-e768-428f-aa72-fc256851b226

📥 Commits

Reviewing files that changed from the base of the PR and between 8dc7373 and 14513ef.

📒 Files selected for processing (3)

• tests/fakes.py
• tests/test_mcp_server.py
• tests/test_navigator_client.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds a shared async helper for pending close task assertions. The MCP server and navigator client tests use the helper instead of duplicating task lifecycle checks.

Changes

Pending close task assertions

Layer / File(s)	Summary
Shared pending close task helper tests/fakes.py	Adds an async helper that waits for task startup, releases the task, awaits completion, and verifies closure and cleanup.
Regression test integration tests/test_mcp_server.py, tests/test_navigator_client.py	Both regression tests import and use the shared helper.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 14513

This test-only refactor centralizes an existing regression assertion without changing production behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: juanpin

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: extracting the shared async test helper.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/admiring-hawking-ehmk8p

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}