Skip to content

test: scale aggregation timings 10x to speed up timing-dependent tests#1759

Merged
bdraco merged 2 commits into
masterfrom
refactor/speed-aggregation-tests
May 20, 2026
Merged

test: scale aggregation timings 10x to speed up timing-dependent tests#1759
bdraco merged 2 commits into
masterfrom
refactor/speed-aggregation-tests

Conversation

@bdraco

@bdraco bdraco commented May 20, 2026

Copy link
Copy Markdown
Member

Summary

Speeds up the top two entries on the slow-test list in #1707 by scaling the multicast aggregation / network-protection constants 10x in the timing tests, in lock-step with the corresponding test sleeps. Each test drops from ~3s to ~0.5s; ~5s saved per test-suite run.

Details

test_response_aggregation_timings and test_response_aggregation_timings_multiple exercise the MulticastOutgoingQueue aggregation window, network protection (~1s), and protected aggregation. The behaviour under test is the ratio of these timings (aggregation completes within window, protection blocks for the full second, protected aggregation collects within its window), not the absolute wall-clock values — so scaling them down together preserves the test contract.

  • New quick_aggregation_timing fixture in tests/conftest.py patches zeroconf._core._AGGREGATION_DELAY (500ms → 50ms), _PROTECTED_AGGREGATION_DELAY (200ms → 20ms), and _ONE_SECOND (1000ms → 100ms). The patches must be in place before AsyncZeroconf is constructed since MulticastOutgoingQueue reads the constants at init time and stashes them on the instance.
  • Each test additionally pins its per-queue jitter (_multicast_delay_random_min / _max, exposed via cdef public in multicast_outgoing_queue.pxd) to 1-5ms so the scaled-down window is not dominated by the unscaled 20-120ms random delay.
  • All asyncio.sleep(N) values are scaled in lock-step with the timing constants; comments now show the scaled arithmetic.

Test plan

  • REQUIRE_CYTHON=1 poetry install --only=main,dev && poetry run pytest tests/test_handlers.py::test_response_aggregation_timings tests/test_handlers.py::test_response_aggregation_timings_multiple --no-cov --timeout=60: 2 passed in 1.14s (was ~6s combined).
  • REQUIRE_CYTHON=1 ... poetry run pytest tests/ --no-cov --timeout=60: 384 passed, 3 skipped in 35.39s (was ~40s).
  • SKIP_CYTHON=1 poetry run pytest tests/ --no-cov --timeout=60: 384 passed, 3 skipped in 35.44s.
  • poetry run pre-commit run on touched files: ruff, ruff format, mypy, codespell, flake8 all green.

Scope

This PR covers the top two slow tests on the list. The next two
(test_get_info_suppressed_by_question_history, test_we_try_four_times_with_random_delay) are dominated by _DUPLICATE_QUESTION_INTERVAL = 999ms, which is cdef'd in src/zeroconf/_services/info.pxd and therefore unpatchable from Python under the Cython build. Speeding those up needs either a cdef-restructure or a virtual-clock approach, and is left as a separate refactor.

`test_response_aggregation_timings` and
`test_response_aggregation_timings_multiple` exercise the
`MulticastOutgoingQueue` aggregation window, network protection
(~1s), and protected aggregation. The behaviour under test is the
ratio of these timings — not their absolute wall-clock values — so
scaling the constants and the corresponding `asyncio.sleep`s down
together preserves the test contract while dropping each test from
~3s to ~0.5s.

Add `quick_aggregation_timing` to `tests/conftest.py` patching
`zeroconf._core._AGGREGATION_DELAY` (500ms → 50ms),
`_PROTECTED_AGGREGATION_DELAY` (200ms → 20ms), and `_ONE_SECOND`
(1000ms → 100ms) before `MulticastOutgoingQueue` is constructed.
Each test additionally pins its per-queue jitter
(`_multicast_delay_random_min` / `_max`, exposed via `cdef public`
in `multicast_outgoing_queue.pxd`) to 1-5ms so the scaled-down
window is not dominated by the unscaled 20-120ms random delay.

Net: ~5s saved per test run; sister tests still cover the same
timing semantics. Closes the top two slowest entries on issue
#1707.

The remaining slow tests in #1707
(`test_get_info_suppressed_by_question_history`,
`test_we_try_four_times_with_random_delay`) are dominated by
`_DUPLICATE_QUESTION_INTERVAL = 999ms` which is `cdef`'d in
`_services/info.pxd` and therefore unpatchable from Python under the
Cython build; speeding those up is a separate refactor.
@codspeed-hq

codspeed-hq Bot commented May 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 14 untouched benchmarks


Comparing refactor/speed-aggregation-tests (ad1c35e) with master (343dc7a)

Open in CodSpeed

@codecov

codecov Bot commented May 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.77%. Comparing base (343dc7a) to head (ad1c35e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1759   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          33       33           
  Lines        3509     3509           
  Branches      493      493           
=======================================
  Hits         3501     3501           
  Misses          5        5           
  Partials        3        3           

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bdraco bdraco marked this pull request as ready for review May 20, 2026 19:30
@bdraco bdraco merged commit 3e5ac4f into master May 20, 2026
36 checks passed
@bdraco bdraco deleted the refactor/speed-aggregation-tests branch May 20, 2026 19:30
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