Skip to content

test: fix flaky test_service_browser_expire_callbacks - #1879

Merged
bdraco merged 2 commits into
masterfrom
fix_expire_callbacks_flake
Aug 29, 2026
Merged

bdraco merged 2 commits into
masterfrom
fix_expire_callbacks_flake

Conversation

@bdraco

@bdraco bdraco commented Aug 29, 2026

Copy link
Copy Markdown
Member

Summary

test_service_browser_expire_callbacks intermittently misses the remove callback (seen on the PyPy 3.10 / skip_cython runner in this run). The test restamped every cached record's TTL from the main thread while the reaper runs DNSCache.async_expire on the event loop thread, and then waited a fixed 0.3 s for each callback.

Details

  • _async_set_created_ttl rewrites the expiration heap; async_expire pops it. Doing the rewrite from the main thread races the reaper, and a re-add that lands mid-pop can leave the record's heap entry behind so the expiry never fires. The restamp now runs on the loop thread via call_soon_threadsafe and blocks on an Event until it has completed. This lives in tests/__init__.py as _restamp_cache, next to _backdate_cache, with a docstring saying which to reach for: _backdate_cache only shifts created (for the RFC 6762 §10.2 recency checks), _restamp_cache re-adds through _async_set_created_ttl so the reaper actually expires the records.
  • The three for _ in range(30): sleep(0.01) loops are replaced with a predicate-based _wait_for(predicate, timeout=2.0) helper in tests/__init__.py, so a slow interpreter or a loaded runner no longer trips the assertion. The happy path is unchanged: it returns as soon as the predicate holds. The other hand-rolled poll loops in test_core.py, test_handlers.py and utils/test_asyncio.py could adopt it in a follow-up; left alone here to keep this a flake fix.

Test plan

  • poetry run pytest tests/services/test_browser.py -k expire_callbacks 10× in a row, SKIP_CYTHON=1
  • tests/services/test_browser.py and tests/test_core.py pass
  • pre-commit run --files tests/services/test_browser.py clean

@github-actions

Copy link
Copy Markdown

Thanks for contributing! A quick note: this project is in the process of moving from LGPL-2.1 to Apache 2.0, see #1835.

By continuing with this pull request you are okay with your contribution, and your previous contributions to this repository, being offered under Apache 2.0.

If that does not work for you, no hard feelings; reply with the exact phrase I do not agree to the Apache 2.0 license or simply close this pull request, and nothing of yours will be included in the change.

@bdraco
bdraco force-pushed the fix_expire_callbacks_flake branch from b77017f to 21c04fd Compare August 29, 2026 23:00
@codspeed

codspeed Bot commented Aug 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing fix_expire_callbacks_flake (21c04fd) with master (250ca7c)

Open in CodSpeed

@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.87%. Comparing base (250ca7c) to head (21c04fd).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1879   +/-   ##
=======================================
  Coverage   99.87%   99.87%           
=======================================
  Files          33       33           
  Lines        3882     3882           
  Branches      567      567           
=======================================
  Hits         3877     3877           
  Misses          3        3           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bdraco
bdraco merged commit 6d061a4 into master Aug 29, 2026
35 checks passed
@bdraco
bdraco deleted the fix_expire_callbacks_flake branch August 29, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant