Skip to content

test(channels): give the Telegram polling test a 30 s ceiling - #483

Merged
itsskofficial merged 1 commit into
existence-master:mainfrom
ShryukGrandhi:fix/issue-420-flaky-test-test-polling-backoff-recovery
Oct 21, 2026
Merged

itsskofficial merged 1 commit into
existence-master:mainfrom
ShryukGrandhi:fix/issue-420-flaky-test-test-polling-backoff-recovery

Conversation

@ShryukGrandhi

@ShryukGrandhi ShryukGrandhi commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What and why

test_polling_backoff_recovery_and_revoked_token gave the whole Telegram polling loop 5 seconds of wall-clock time: six scripted getUpdates replies, four backoffs (fake sleep), one dispatched update and the stop on the 401, each with its own SQLite commits. On a busy Windows runner with -n auto that budget ran out while the loop was waiting on an aiosqlite commit, so the test failed even though the logic was fine.

The loop ends by itself on the revoked token, so the limit is only a guard against a hang. This raises it to 30 s, the same ceiling until() in tests/channels/conftest.py already uses, and removes the @pytest.mark.flaky(reruns=2) marker that was added as a stopgap.

Fixes #420

How I tested it

  • Reproduced the CI failure locally by loading a small throwaway pytest plugin that adds 1 s to every aiosqlite.Connection.commit (standing in for a busy CI worker), with reruns turned off:
    PYTHONPATH=/tmp/slowsql420 .venv/bin/python -m pytest -q tests/channels/test_telegram.py -k polling_backoff -p slowsql -p no:rerunfailures
    • before: TimeoutError at 5.00 s, same as the CI run
    • after: passes, the loop takes about 10 s
  • .venv/bin/python -m pytest -q tests/channels -p no:rerunfailures: 154 passed, 1 skipped. On one earlier run test_voice_note_is_transcribed_and_answered failed once. It passed on its own and on the next full run, and this change doesn't touch it.
  • .venv/bin/ruff check sentient tests packaging: clean

Real test

Test-only change, no model involved. The evidence is the slowed-commit reproduction above, which fails the old 5 s limit and passes the new one. No changelog entry because users won't notice this change.

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 (n/a)
  • A changelog entry in changes/ if users will notice this change (n/a, test only)
  • No keys, tokens or personal data in the code, tests or screenshots

Summary by CodeRabbit

  • Tests
    • Updated polling recovery test timing to allow up to 30 seconds for completion.
    • Removed automatic flaky-test reruns; the test now completes normally upon receiving the expected response.

@github-actions github-actions Bot added the area: channels Telegram, Discord label Oct 20, 2026
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: ed1bee14-836a-4b71-8396-9e851706165c
📥 Commits

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

📒 Files selected for processing: 1
  • tests/channels/test_telegram.py

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

📝 Walkthrough

The Telegram polling recovery test no longer uses a flaky-test rerun annotation. Its ch.run() timeout increases from 5 to 30 seconds, with a comment noting that the polling loop normally ends on the 401 response.

Changes: Telegram polling recovery

Layer / File(s) Summary
Polling recovery test timeout
tests/channels/test_telegram.py
The test removes the flaky rerun annotation and its Windows CI timeout comment. The timeout for ch.run() increases from 5 to 30 seconds. A comment notes that the loop normally ends on the 401 response.

Priority: ⬇️ Low

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

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to e6a88

The polling test retains both its expected termination path and a bounded timeout, with no merge-blocking risk identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 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: increasing the Telegram polling test timeout to 30 seconds.
Linked Issues check Passed The change satisfies issue #420. It increases the polling test timeout from 5 seconds to 30 seconds and removes the temporary pytest.mark.flaky(reruns=2) marker. The test comment confirms that the 4…
Out of Scope Changes check Passed The whole pull request changes only tests/channels/test_telegram.py. Both changes directly support issue #420: the timeout increase provides the required busy-runner budget, and marker removal remov…
Full details: Linked Issues check

Explanation

The change satisfies issue #420. It increases the polling test timeout from 5 seconds to 30 seconds and removes the temporary pytest.mark.flaky(reruns=2) marker. The test comment confirms that the 401 response remains the normal termination condition. These changes address the documented Windows CI timeout without changing production behavior.

Full details: Out of Scope Changes check

Explanation

The whole pull request changes only tests/channels/test_telegram.py. Both changes directly support issue #420: the timeout increase provides the required busy-runner budget, and marker removal removes the documented stopgap. No unrelated production, documentation, or dependency changes are present.

  • 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 polling clock,
Thirty seconds on the test log,
The 401 ends the run,
No flaky reruns in the sun,
I nibble carrots, job well done!

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

@itsskofficial itsskofficial left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ShryukGrandhi! Letting the loop end itself on the revoked token, with a generous ceiling only as a hang guard, is exactly what our flaky-test policy recommends, and removing the marker in the same PR is spot on. I ran it 50 times quiet, 35 times next to the full suite and 64 times at 16 at once, all green. Approving and merging.

@itsskofficial
itsskofficial added this pull request to the merge queue Oct 21, 2026
Merged via the queue into existence-master:main with commit 108fc26 Oct 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: channels Telegram, Discord

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: test_polling_backoff_recovery_and_revoked_token times out on Windows CI

2 participants