Harden AuTests against timing races - #13489
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Hardens flaky AuTest gold tests by reducing timing and framing sensitivity in chunked HTTP/2 origin tests and the stale-response memory-cap test.
Changes:
- Makes netcat-based origins persistent (
nc -lk) to avoid one-shot listener races. - Adds port-readiness gating via
When.PortOpen(...)before starting HTTP/2 clients. - Pads the stale-response header to align with a cap-plus-sentinel read boundary for deterministic memory rejection.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/gold_tests/pluginTest/stale_response/stale_response_max_memory.replay.yaml | Adds a padding header to stabilize the memory-cap rejection boundary. |
| tests/gold_tests/chunked_encoding/server2.sh | Switches netcat listener to keep-open mode for the origin script. |
| tests/gold_tests/chunked_encoding/server3.sh | Switches netcat listener to keep-open mode for the origin script. |
| tests/gold_tests/chunked_encoding/delay-server.sh | Switches netcat listener to keep-open mode for the delayed origin script. |
| tests/gold_tests/chunked_encoding/chunked_encoding_h2.test.py | Adds port-open readiness checks before starting HTTP/2 clients. |
Two AuTests can fail depending on process scheduling and response framing. The HTTP/2 chunked clients can reach their origins before the listeners are ready, while the capped stale-response fetch can stop at a body-block boundary and time out instead of exercising its memory fallback. This patch uses a deterministic raw origin that ignores readiness probes, serves one real request, and exits. It also sizes and documents the stale-response header at the cap-plus-sentinel read boundary so memory-limit rejection is independent of body segmentation.
182852c to
e79079b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/gold_tests/chunked_encoding/chunked_encoding_h2_server.py:53
request += data(and laterbody += data) repeatedly concatenates immutablebytes, which is O(n^2) as the request grows. Switching to abytearray(or collecting chunks in a list andb"".join(...)once) avoids quadratic behavior and makes this helper more robust if a larger request is ever sent during tests.
request = b""
while b"\r\n\r\n" not in request:
data = conn.recv(4096)
if not data:
return request
request += data
tests/gold_tests/chunked_encoding/chunked_encoding_h2_server.py:52
recv()can block indefinitely if a client connects and sends partial headers (or stalls) without closing the connection, which can hang the AuTest run. To make the test harness failure mode bounded, set a reasonable socket timeout onconn(and handlesocket.timeoutby closing/continuing) so the server can recover instead of blocking forever.
while b"\r\n\r\n" not in request:
data = conn.recv(4096)
if not data:
return request
tests/gold_tests/pluginTest/stale_response/stale_response_max_memory.replay.yaml:84
- The hardcoded “215 bytes”/“42 wire bytes” calculations are brittle if header serialization changes (e.g., ordering, quoting, or YAML emitter differences), which could reintroduce flakiness. Consider adding a short note indicating how/where the 256+1 boundary is enforced in the code under test and that
X-Paddingmust be re-tuned if serialization changes; alternatively, setX-Paddinglonger than necessary (with a comment stating the target is “≥ cap+1”) to reduce sensitivity to minor formatting differences.
# The preceding fields serialize to 215 bytes. X-Padding adds 42 wire
# bytes (name, separator, 29-byte value, and CRLF), bringing the header
# to the 256-byte limit plus the one-byte overflow sentinel. This makes
# the memory rejection independent of body segmentation.
- [ X-Padding, aaaaaaaaaaaaaaaaaaaaaaaaaaaaa ]
Two AuTests can fail depending on process scheduling and response framing. The HTTP/2 chunked clients can reach their origins before the listeners are ready, while the capped stale-response fetch can stop at a body-block boundary and time out instead of exercising its memory fallback. This patch uses a deterministic raw origin that ignores readiness probes, serves one real request, and exits. It also sizes and documents the stale-response header at the cap-plus-sentinel read boundary so memory-limit rejection is independent of body segmentation. (cherry picked from commit 80452f0)
|
Cherry-picked to the 10.2.x branch as 2696089 for the 10.2.0 release. |
Two AuTests can fail depending on process scheduling and response
framing. The HTTP/2 chunked clients can reach their origins before the
listeners are ready, while the capped stale-response fetch can stop at
a body-block boundary and time out instead of exercising its memory
fallback.
This patch uses a deterministic raw origin that ignores readiness
probes, serves one real request, and exits. It also sizes and documents
the stale-response header at the cap-plus-sentinel read boundary so
memory-limit rejection is independent of body segmentation.