[itsskofficial]

Closes #422.

tests/migrate/test_memory_import.py::test_routes polled the background ChatGPT import 200 times with 20 ms sleeps, a budget of about 4 seconds. On a busy Windows runner running tests in parallel the import was still running when the budget ran out. The result itself was always right.

Change

• Poll against a 30-second deadline instead of a fixed number of sleeps. A healthy run still finishes in well under a second.
• Drop the @pytest.mark.flaky(reruns=2) marker, so a real failure isn't hidden behind reruns.

Checked

• Ruff is clean.
• The file passes three times in a row locally (52 passed each time).
• CI on Linux and Windows below.

Test-only change, so no changelog entry.

Summary by CodeRabbit

• Tests
  • Updated migration import-job checks to wait for the job to finish, with a 30-second deadline, rather than stopping after a fixed number of requests.
  • Removed the flaky-test marker from the route test.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: f4c565d1-9982-4a98-8a1a-9da4f7d64cac

Commits

Reviewing files that changed from the base of the PR and between 91053cc and 6555f2c.

Files selected for processing: 1

• tests/migrate/test_memory_import.py

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

Walkthrough

The test_routes test now polls the import job until it finishes or 30 seconds pass. The test no longer uses the flaky-test marker.

Changes: Import job test polling

Layer / File(s)	Summary
Poll for import completion tests/migrate/test_memory_import.py	The test polls while the job is running and the 30-second deadline has not passed. It also removes the flaky-test marker.

Priority: Low

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

Change: Other · Severity of issue fixed: Low

Suggested labels: area: memory, no changelog

Merge Risk: Minimal · up to 6555f

The test change safely extends polling for slow imports and preserves failure reporting on timeout.

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 1 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 and concisely describes the main change: updating test_routes to wait up to 30 seconds for the import job.
Linked Issues check		Issue #422 asks for the wait in tests/migrate/test_memory_import.py::test_routes to use a deadline (for example 30 s) or the job's completion event, and says the result itself was correct. The diff re…
Out of Scope Changes check		The whole-PR diff against the merge base 91053cc touches one file, tests/migrate/test_memory_import.py, with +2/-5 lines. Every hunk serves #422: the deadline-based polling loop, the explanatory comme…

Full details: Linked Issues check

Explanation

Issue #422 asks for the wait in tests/migrate/test_memory_import.py::test_routes to use a deadline (for example 30 s) or the job's completion event, and says the result itself was correct. The diff replaces the fixed for _ in range(200) loop with a time.monotonic() deadline of 30 seconds. The loop polls GET /api/import/memory/job every 20 ms while status is running and the deadline has not passed. The final assertion (now["id"] == job["id"] and now["status"] == "done" and now["facts"] == 3) is unchanged, so the result check still applies. The @pytest.mark.flaky(reruns=2) marker that #422 describes as a temporary workaround is removed. import time is already present at the head. The walrus assignment in the while condition sets now on the first evaluation, so the assertion after the loop always has a value. I did not run the test. The PR reports three local passes and Ruff clean, which I did not verify.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base 91053cc touches one file, tests/migrate/test_memory_import.py, with +2/-5 lines. Every hunk serves #422: the deadline-based polling loop, the explanatory comment that cites #422, and removal of the flaky marker that #422 says was a temporary workaround. No production code, fixtures, or other tests changed. The PR description states no changelog entry, which fits a test-only change.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

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

_{A rabbit checks the clock with care,
The import job still runs somewhere.
It waits until the task is through,
Or thirty seconds have passed on cue.
No flaky tag remains to spare.}

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