tests: stabilize flaky tests in CI - #2523
Open
tzssangglass wants to merge 3 commits into
Open
Conversation
Contributor
Author
|
Lint PR should be fixed by:#2522 |
tzssangglass
force-pushed
the
fix-socket-tests-dns
branch
from
September 7, 2026 13:18
7d3d437 to
6aed999
Compare
Public recursive resolvers (8.8.8.8 et al) no longer answer A queries for "localhost." — RFC 6761 special-use handling pushes that resolution back to stub resolvers (/etc/hosts), so nginx's `resolver` directive, which queries the recursive server directly, now fails with "localhost could not be resolved (3: Host not found)".
…n-abort.t t/189 died mid-file with "IPC::Run: timeout on timer" and exit status 29: test-nginx arms IPC::Run::timeout() with the same value as curl's --max-time, and the IPC::Run timer checks integer-second time() with a 1s fudge, so when curl's launch phase crosses a second boundary the harness kills curl before its own --max-time fires and croaks, aborting the whole file and leaving shutdown_error_log checks to fail spuriously. Since the harness timer cannot be widened from the test side, make curl self-expire strictly earlier instead: append "--- curl_options: --max-time=0.9" (curl_options is appended after the scaffold's own --max-time, so curl honors the later, smaller value), leaving the IPC::Run timer more than a second of slack. t/024-access/on-abort.t TEST 7 reads the error.log exactly once after --- wait: 0.2 — the whole budget for abort detection plus the cosocket roundtrip to redis; on loaded runners this window is regularly blown. Bump the wait to 1s. Flaky CI evidence: https://github.com/openresty/lua-nginx-module/actions/runs/34126708814/job/101756976592?pr=2523 https://github.com/openresty/lua-nginx-module/actions/runs/34126708814/job/101756976782?pr=2523 Signed-off-by: tzssangglass <tzssangglass@gmail.com>
tzssangglass
force-pushed
the
fix-socket-tests-dns
branch
from
September 8, 2026 14:26
6aed999 to
7bd03b4
Compare
The mock TCP server flushes the received query to tcp_query_file after every recv (Util.pm atomic-rename trick) and the test side reads the file exactly once after --- wait: 0.05. TEST 4 sends "flush_all\r\n" one byte at a time with 1ms sleeps, so on a loaded runner the forked mock server gets starved between recvs and the check samples the file while it still holds a 7- or 10-byte prefix (got 'flush_a', expected 11 bytes). Give the mock server 0.5s to drain the kernel buffer. Flaky CI evidence: https://github.com/openresty/lua-nginx-module/actions/runs/34238302357/job/102101640297?pr=2523 https://github.com/openresty/lua-nginx-module/actions/runs/34238302357/job/102101640217?pr=2523 Signed-off-by: tzssangglass <tzssangglass@gmail.com>
Contributor
Author
|
@zhuizhuhaomeng PTAL |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tests: fix flaky tests in CI
This PR fixes the flaky tests that keep showing up in the CI matrix, with root-cause analysis and local reproduction for each.
t/014-bugs.t, t/058-tcp-socket.t — resolver fails on "localhost"
Symptom:
localhost could not be resolved (3: Host not found).Root cause: public recursive resolvers (8.8.8.8 et al) no longer answer A queries for
localhost.— RFC 6761 special-use handling pushes that resolution back to stub resolvers (/etc/hosts), but nginx'sresolverdirective queries the recursive server directly.Fix: resolve
sslip.io(answerable by public resolvers) instead oflocalhost.t/189-http2-subreq-error-wakeup.t — file dies with "IPC::Run: timeout on timer" (exit 29)
Evidence: job 101756976592
Root cause: test-nginx arms
IPC::Run::timeout()with the exact same value it passes to curl's--max-time, and the IPC::Run timer checks integer-secondtime()with a 1s fudge (IPC/Run/Timer.pm:end_time = start_time + interval + 1), while curl's clock starts a few milliseconds later (after fork+exec). When curl's launch phase crosses a whole-second boundary — easy on a loaded runner — curl's deadline lands after the harness timer'send_time, so the harness kills curl before its own--max-timefires and croaks, aborting the whole test file (Test::Builder exits with the failed-subtest count, e.g. 29) and leaving--- shutdown_error_logchecks to fail spuriously.Fix:
--- curl_options: --max-time=0.9in the three timeout blocks.curl_optionsis appended after the scaffold's own--max-time, so curl honors the later, smaller value and always self-expires strictly before the harness timer — the race window disappears. A general fix for test-nginx itself (giveIPC::Run::timeoutslack over--max-time) will be submitted upstream separately.t/024-access/on-abort.t TEST 7 — "callback done: +OK" missing
Evidence: job 101756976782
Root cause: the error.log check is one-shot (no polling) and
--- wait: 0.2was the entire budget for abort detection plus the on_abort callback's cosocket roundtrip to redis (~0.22s). On loaded runners this window is regularly blown (locally: a 220ms redis reply still passes, 260ms reliably fails).Fix: bump
--- waitto 1s.t/128-duplex-tcp-socket.t TEST 4 — tcp_query truncated (7/10 of 11 bytes)
Evidence: job 102101640297, job 102101640217
Root cause: the test sends
flush_all\r\none byte at a time (1ms sleeps); the mock TCP server flushes the received query totcp_query_fileafter every recv (atomic rename), and the test side reads the file exactly once after--- wait: 0.05. On a loaded runner the forked mock server gets starved between recvs, so the check samples the file while it still holds a 7- or 10-byte prefix (got 'flush_a', expected 11 bytes).Fix: bump
--- waitto 0.5s so the mock server can drain the kernel buffer.Known remaining flake (not addressed here)
t/109-timer-hup.tTEST 3 occasionally flips both timer assertions when the HUP reload is slow enough that the old worker's 3s timer expires normally (prematurely expired: false) instead of being aborted. This is a HUP-timing assumption baked into the test, orthogonal to the fixes above; keeping it under observation for now.I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.