Skip to content

Fix flaky test_ash_end_to_end - #760

Merged
puddly merged 1 commit into
devfrom
zigpy-bot/deflake-ash-end-to-end
Sep 28, 2026
Merged

puddly merged 1 commit into
devfrom
zigpy-bot/deflake-ash-end-to-end

Conversation

@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

test_ash_end_to_end fails intermittently on CI, most recently on dev in https://github.com/zigpy/bellows/actions/runs/36362650409 (Python 3.14, FakeTransportWithDelays: TimeoutError instead of NotAcked), and earlier in https://github.com/zigpy/bellows/actions/runs/34772581160 and https://github.com/zigpy/bellows/actions/runs/30421043625 (DID NOT RAISE TimeoutError at the "ncp failure" step, fixed by a rerun).

The test scales the ASH ACK timeouts down to 0.4–3.2 ms and used wall-clock sleeps to decide how long a link stays down, so a short CPU stall on a busy runner changes the outcome. This is a test-only change; bellows/ash.py is untouched.

  • "host failure" / "ncp failure" steps: the transport was unpaused after a fixed sleep(t_rx_ack * 15). If the last of the 5 attempts had not been sent yet by then, it got through and no TimeoutError was raised. The link now stays paused until the send has actually failed.
  • Reset: a fixed sleep(0.01) after sending RST did not guarantee the RSTACK had been processed, so the next send could hit NcpFailure. The test now waits (bounded to 1 s) until the host is connected again.
  • Excessive-NAK step: each NAK raced a sub-millisecond host ACK timeout. If the final attempt timed out first, TimeoutError came out instead of NotAcked (the failure in the linked run). The host now gets a 5 s ACK timeout for this step, and FakeTransportRandomLoss drops nothing during it, so every attempt ends in a NAK. The NAK retry and final-NAK paths in _send_data_frame are still covered (ash.py coverage unchanged).

Stress results with pytest-repeat + pytest-xdist on an 8-core machine, test_ash_end_to_end only:

Load dev this PR
-n 8, 6000 runs (CI-like) 2 failed 0 failed
-n 24, 1600 runs (3× oversubscribed) 178 failed 0 failed

Under even heavier load (-n 32, 4000 runs) this PR still failed 2 runs. Both were sends that are expected to succeed but ran out of their ~14 ms retry budget. Fixing that would mean raising the patched T_RX_ACK_MAX, which made each parametrization about 5× slower (≈0.3–0.45 s), so it is left out.

The test scales the ASH ACK timeouts down to milliseconds and used wall-clock
sleeps to decide how long a link stays down, so a CPU stall on a busy CI runner
could flip its outcome:

- The "host failure" and "ncp failure" steps unpaused the transport after a
  fixed sleep. If the retries had not all timed out yet, the next one got
  through and the expected TimeoutError never came. The link now stays down
  until the send has failed.
- After sending RST, a fixed 10 ms sleep did not guarantee the RSTACK had been
  processed. Wait for the host to be connected again instead.
- The excessive-NAK step raced each NAK against a sub-millisecond ACK timeout;
  if the final attempt timed out first, TimeoutError was raised instead of
  NotAcked. Give the host a 5 s ACK timeout and disable random frame loss for
  that step, so every attempt ends in a NAK.
@codecov

codecov Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.58%. Comparing base (c93fc1b) to head (0b3d19f).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #760   +/-   ##
=======================================
  Coverage   99.58%   99.58%           
=======================================
  Files          64       64           
  Lines        4330     4330           
=======================================
  Hits         4312     4312           
  Misses         18       18           

☔ 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.

@puddly
puddly merged commit 9c4b89a into dev Sep 28, 2026
22 checks passed
@puddly
puddly deleted the zigpy-bot/deflake-ash-end-to-end branch September 28, 2026 01:06
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.

2 participants