[ShryukGrandhi]

What and why

test_stalled_download_cancel_is_bounded_and_partial_file_is_removed failed once on Windows CI with TimeoutError. The test sets DOWNLOAD_MAX_DURATION_S and DOWNLOAD_CANCEL_TIMEOUT_S to 0.01 s, which are the limits it actually checks, but wrapped the whole save, cancel attempt and partial-file removal in a 0.5 s asyncio.wait_for. On a loaded runner with -n auto the event loop can stall for longer than that, so the outer guard fired before _save_download raised the expected BrowserError.

The outer guard is only there to stop the test hanging if the cancel bound breaks, so it is now 10 s. The inner 0.01 s limits and the asserted error message are unchanged, so a cancel that is not bounded still fails the test (it would hit the guard and raise TimeoutError instead of BrowserError). The @pytest.mark.flaky(reruns=2) marker is removed, as docs/DEVELOPING.md asks for in the same PR as the fix.

Fixes #421

How I tested it

• Reproduced the failure with a scratch copy of the test whose save_as blocks the event loop for 0.6 s (standing in for a busy runner): with the 0.5 s guard it fails with the same TimeoutError as CI, and with the 10 s guard it passes and still gets the expected BrowserError("Browser cancellation failed: timed out after 0.01 seconds").
• pytest -q tests/browser/test_profiles.py: 73 passed
• pytest -q -n auto tests/browser: 137 passed
• ruff check sentient tests packaging: clean

Real test

Test-only change, no engine or model involved. Checked on macOS (arm64); I have no Windows runner, so the proof there is CI not needing a rerun for this test.

Checklist

• Tests added or updated, and pytest / npm run typecheck pass locally for what I touched
• ruff check sentient tests is clean
• docs/API.md updated if the desktop ↔ engine contract changed (not changed)
• A changelog entry in changes/ if users will notice this change (test-only, users won't notice)
• No keys, tokens or personal data in the code, tests or screenshots

Summary by CodeRabbit

• Tests
  • Improved test reliability by increasing the test’s overall timeout.
  • Clarified that the short cancellation limits remain the behavior being verified.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3dc3b3c2-810d-4a9f-bc0c-e3e9c9202f71

📥 Commits

Reviewing files that changed from the base of the PR and between ee7a684 and b43b2da.

📒 Files selected for processing: 1

• tests/browser/test_profiles.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

The stalled-download cancellation test no longer has a flaky-test marker. Its outer timeout is now 10 seconds, while the 0.01-second cancellation limits remain the behavior under test.

Changes: Stalled-download test

Layer / File(s)	Summary
Test timeout and retry behavior tests/browser/test_profiles.py	The test no longer uses pytest.mark.flaky(reruns=2). Comments clarify the cancellation limits, and the outer asyncio.wait_for timeout increases from 0.5 to 10 seconds.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to b43b2

The test preserves its cancellation and cleanup coverage while allowing slower CI runners more time to complete.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: increasing the stalled download test’s hang guard for busy runners.
Linked Issues check		Issue #421 asks for a longer outer hang guard while preserving the 0.01-second cancellation limits and detecting unbounded cancellation, then removing the flaky marker. The test raises the outer timeo…
Out of Scope Changes check		The only changes in tests/browser/test_profiles.py address issue #421. The timeout comment supports the test change. No unrelated changes appear in the reviewed diff.

Full details: Linked Issues check

Explanation

Issue #421 asks for a longer outer hang guard while preserving the 0.01-second cancellation limits and detecting unbounded cancellation, then removing the flaky marker. The test raises the outer timeout from 0.5 to 10 seconds. It keeps both inner limits and the expected BrowserError assertion unchanged, and removes @pytest.mark.flaky(reruns=2).

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the timeout clock,
While tiny limits guard the test.
The flaky tag hops out the door,
Ten seconds give the task more rest.
The partial file is checked once more,
Then quiet paws hop off to bed.}

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

[itsskofficial]

Thanks @ShryukGrandhi! The comment explaining that the 0.01 s limits are what's under test is a nice touch, and a cancel that never ends still fails the test. I ran it 50 times quiet, 140 times next to the full suite and 64 times at 16 at once, all green. Approving and merging.