Skip to content

test(windows): assert what the server did, not what the client said - #308

Merged
EdmondDantes merged 2 commits into
mainfrom
windows-tests-assert-the-server
Aug 26, 2026
Merged

EdmondDantes merged 2 commits into
mainfrom
windows-tests-assert-the-server

Conversation

@EdmondDantes

Copy link
Copy Markdown
Contributor

Closes #307.

The three tests still failing on WINDOWS_X64_ZTS_RELEASE each assert something
other than what the server did. The server's answers in all three artifacts are
correct.

compression/060 named two %Agarbage: 400 lines while the body prints one
per encoding the build has. SKIPIF skips only when neither brotli nor zstd is
built, so a build with exactly one — the Windows build has brotli — ran and
failed every time. The per-encoding lines stay, for diagnosis, and the verdict
is a single line that holds whatever the build contains.

chaos/002 floods 2000 CONTINUATION frames at a server that is entitled to
tear the connection down mid-flood; the test says so in its own comment and
wraps the flood in try/catch. H2TestClient::writeFrame ignored what fwrite
returned and kept writing into a reset socket, and on Windows each of those
raised a warning into the transcript — nine of them. fwrite warns rather than
throwing, so the catch never fired. The client latches the gone peer and
throws once.

tls/006 read keep-alive out of curl's verbose log, counting
Re-?using existing connection. That wording has changed between curl releases,
and the log cannot distinguish "curl opened three connections" from "curl said
it differently". The server sees the answer directly: three requests on one
kept-alive connection carry one peer port. HttpRequest::getRemotePort()
replaces the log scrape, and the output line is connections: 1 rather than
reuses: 2. Same move as #299 for static/013.

Evidence

Locally, with openssl on PATH so the TLS group executes: the whole suite is 496
tests, 259 executed, 0 failed, 0 warned — up from 245 executed, because the
TLS group runs. tls/006 and chaos/002 pass 4 of 4 runs each with the new
assertions.

chaos/002 and tls/006 pass on this machine before the change as well: what
they measured was the client's phrasing and the platform's warning, so the
reproduction is the Windows job, not this box. 060 skips here — neither
encoding is built — and its defect is arithmetic in the file: two expected lines
against one echo per built encoding, with the CI .out showing exactly one.

The Windows job is the proof for all three.

…307)

060 named two output lines while printing one per built encoding, so a
build with brotli and no zstd failed for something the server got right.
It now reports a verdict that does not depend on how many encodings the
build has.

chaos/002 drove H2TestClient into a socket the server had reset, and
every further fwrite() raised a warning into the test transcript. The
client latches the dead peer and throws once, which is what the flood
loop already catches.

tls/006 counted "Re-using existing connection" in curl's verbose log,
wording that changes between curl releases. Keep-alive is now read from
the peer port the server itself sees, one per request.
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Coverage

Total lines: 83.17% → 83.15% (-0.02 pp)

File Baseline Current Δ Touched
src/http3/http3_callbacks.c 83.54% 83.44% -0.11 pp
src/http3/http3_listener.c 77.36% 77.96% +0.60 pp
src/http3/http3_packet.c 90.43% 85.22% -5.22 pp
src/websocket/php_websocket.c 78.61% 78.44% -0.17 pp

collectResponse writes SETTINGS and PING acks and window refills from
inside its read loop, and nine call sites destructure its tuple without
a catch. A server that closes right after sending one of those frames
would have reached them as an uncaught exception where a closed
connection used to give them an empty response. Frames the caller asked
for still throw; the ones the client emits by itself latch and return
false.

chaos/001 drives the same open-then-RST burst chaos/002 does, with no
catch: the day the server answers rapid reset by closing, that loop
breaks for the reason chaos/002 broke.

The write-failure message no longer claims the peer closed, since a
send timeout reaches the same branch.
@EdmondDantes
EdmondDantes merged commit e2493d7 into main Aug 26, 2026
7 of 9 checks passed
@EdmondDantes
EdmondDantes deleted the windows-tests-assert-the-server branch August 26, 2026 08:32
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.

The three remaining Windows failures are all in the tests, not the server

1 participant