test(windows): assert what the server did, not what the client said - #308
Merged
Merged
Conversation
…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.
Contributor
CoverageTotal lines: 83.17% → 83.15% (-0.02 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.
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.
Closes #307.
The three tests still failing on
WINDOWS_X64_ZTS_RELEASEeach assert somethingother than what the server did. The server's answers in all three artifacts are
correct.
compression/060named two%Agarbage: 400lines while the body prints oneper 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/002floods 2000 CONTINUATION frames at a server that is entitled totear the connection down mid-flood; the test says so in its own comment and
wraps the flood in
try/catch.H2TestClient::writeFrameignored whatfwritereturned and kept writing into a reset socket, and on Windows each of those
raised a warning into the transcript — nine of them.
fwritewarns rather thanthrowing, so the
catchnever fired. The client latches the gone peer andthrows once.
tls/006read keep-alive out of curl's verbose log, countingRe-?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: 1rather thanreuses: 2. Same move as #299 forstatic/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/006andchaos/002pass 4 of 4 runs each with the newassertions.
chaos/002andtls/006pass on this machine before the change as well: whatthey measured was the client's phrasing and the platform's warning, so the
reproduction is the Windows job, not this box.
060skips here — neitherencoding is built — and its defect is arithmetic in the file: two expected lines
against one
echoper built encoding, with the CI.outshowing exactly one.The Windows job is the proof for all three.