test(fpc): keep-alive and WebSocket EAGAIN regression tests - #564
Open
freitasjca wants to merge 1 commit into
Open
test(fpc): keep-alive and WebSocket EAGAIN regression tests#564freitasjca wants to merge 1 commit into
freitasjca wants to merge 1 commit into
Conversation
Two standalone FPC programs covering transports the DUnitX suite cannot reach, since it is Delphi-only. Neither depends on a test framework, and each exits non-zero on failure. FPCHttpKeepaliveTest.dpr drives 30 sequential requests over one reused connection and asserts each completes within 35 ms. KeepConnection := True is required: TFPHTTPClient defaults it to False, and the stall only appears on a reused connection because Linux quickack makes the first ACK on every socket immediate. It refuses to build below FPC 3.3.1, where the provider's keep-alive and TCP_NODELAY code is not compiled at all. Verified against 3.3.4 on trunk: max 1 ms. tests/fpc-websocket/ covers the EAGAIN path on the epoll transport. It asserts SILENCE after the first EAGAIN rather than liveness, which is the distinction that catches the defect. 5/5 on FPC 3.2.2 against the reworked implementation in 3.3.4. Additive only: no existing file is modified except .gitignore, which gains two entries for FPC test binaries whose Linux builds have no extension.
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.
Two standalone FPC regression tests for transports the DUnitX suite cannot reach.
Horse's own test suite is DUnitX and Delphi-only, so the FPC keep-alive path and
the FPC + epoll WebSocket transport have no way to be covered from it. Both of
these are plain FPC programs with no test-framework dependency, and each exits
with a non-zero code on failure so they gate directly.
Purely additive: three new files plus two
.gitignoreentries. Nothing existingis modified, and neither test is wired into CI — that seemed better left to you
to do the way you prefer.
tests/src/FPCHttpKeepaliveTest.dpr— keep-alive latency (#562)30 sequential GETs on one reused connection, asserting each completes within
35 ms. Verified against 3.3.4 on FPC trunk: max 1 ms, avg 0 ms.
Two details are load-bearing rather than incidental:
KeepConnection := Trueon the client.TFPHTTPClient.KeepConnectiondefaults to
False, and the stall only occurs on a reused connection — Linuxstarts every socket in quickack mode, so the first ACK is immediate and Nagle
never engages. An earlier version of this test omitted it, opened a fresh
connection per request, and passed identically with and without the fix.
A compile-time refusal below FPC 3.3.1. The provider's keep-alive and
TCP_NODELAYhandling sit behind{$IF FPC_FULLVERSION >= 30301}, so on 3.2.2there is nothing to test: the server closes after each response and the run dies
with
Error reading data from socket, an error that names nothing relevant.Failing at compile time says so instead.
The header documents the cause as Nagle interacting with the peer's delayed ACK,
per the wire capture on #562 — the body segment leaves 11-14 µs after the ACK
arrives, across an ACK delay that itself varies 42.5-43.9 ms.
This complements
tests/fpc_keepalive_regression.pyrather than replacing it.That one drives
FPCHttpKeepAliveServer.dprfrom Python; this one isself-contained Pascal and runs where Python is not installed.
Build and run:
tests/fpc-websocket/— WebSocket EAGAIN (#549)Written during the review of #549 and kept afterwards. 5 checks, all passing
on FPC 3.2.2 against the reworked implementation now in 3.3.4 — errno captured
once and passed to both predicates, close deferred via
FStateLockplus anFActiveOperationsrefcount. That is code this test was not written for, whichis the useful part: it asserts a property rather than an implementation.
The load-bearing assertion is silence, not liveness. epoll sets
O_NONBLOCKon accepted sockets, so
recvreturns-1/EAGAINwhenever no frame has arrivedyet. The original defect read that as a disconnect, broke the upgrader's read
loop on its first iteration, and let the HTTP pipeline resume — writing a stray
HTTP/1.1 200 OKonto a socket already handed to WebSocket. An earlier draftasserted only that the connection was still open, and passed against the broken
code.
The runner refuses to build a tree lacking
WS_SOCKET_READ_TICK_MS, so itcannot report a pass on a checkout predating the fix.
Exit code is the number of failed checks.
.gitignoreTwo entries for the FPC test binaries. Their Linux builds have no extension, so
the existing
*.exerule does not cover them. One of the two isFPCHttpKeepAliveServer— your own file, which produces the same uncoveredartefact when built on Linux.
Happy to adjust naming, placement, or wire either into the Docker test image if
you would like them in CI.