Uh oh!
There was an error while loading. Please reload this page.
test(h2): cover the emit-mode pin and cite the half-close test (#224, #225) - #252
Merged
Conversation
…225) The mode choice moves out of h2_session_emit_ex into h2_tls_emit_use_drain, compiled with or without OpenSSL so the unit suite links it. Four cases in HTTP2Strategy cover it, and removing the pin turns exactly one of them red. The wire shape stays untested: it needs a frame-level TLS client the suite does not have. #225 was already covered by h2/064 and the CHANGELOG now says so — with the re-drive removed from the completion tail that test fails 10 runs of 10, all 1048576 body bytes and no END_STREAM.
Contributor
CoverageTotal lines: 83.02% → 83.02% (-0.00 pp)
|
…sion http_protocol_strategy_destroy did one efree while the HTTP/2 and WebSocket strategies each allocate a session lazily and release it from cleanup(conn), reachable only through conn->strategy. A dispose(strategy) op carries the release now, destroy calls it, and cleanup routes to the same code. Evidence: the HTTP2Strategy unit binary reported 184 bytes over two blocks and reports none now. ctest 16 of 16, 463 of 487 phpt (24 skipped, 0 failed).
… produce The expectation read ret=1, counting only the remote post. HttpServer::send() returns room_hub_send's delivered, which is out.served + out.posted — every subscriber this worker served plus one post per remote worker — and the split of 12 connections over two workers is not fixed, so the figure ranged 4 to 9 across runs. The test now bounds it. SUBS also had to travel bound to the closure: the worker has its own constants, and reading SUBS there threw an Error the catch reported as exc=Error. The test had never run: it is gated on --enable-tas-test-hooks, which no build here or in CI configures. Verified with that flag on, 10 runs of 10; whole tree 467 of 487 (20 skipped, 0 failed).
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Both entries closed their fixes with "carried without a test". Neither claim survived being checked.
#225 was already covered
h2/064builds the peer the entry asked for: a half-close with a writev in flight defers the destroy, and the re-drive inhttp_send_batched_finishis what gets the last frames out. Put that completion tail back the way it was before #223 and the test fails 10 runs of 10 — all 1048576 body bytes arrive, END_STREAM does not, so the peer cannot tell a finished body from a cut one. That is the entry's own description of the defect. It passes 10 of 10 with the tail in place. No code changed for this; the CHANGELOG now cites the test.#224 could not get the test its entry imagined
The parked slice appears only in DRAIN, and DRAIN is picked when
large_streams_pending == 0— but a body overH2_TLS_HYBRID_LARGE_THRESHOLD(2048, strict>) raises that counter itself, so no single large response reaches the window. It takes a dozen streams of exactly 2048 bytes behind a zero initial window, then one TCP write carrying the WINDOW_UPDATE batch plus a request for a large body — at frame level over TLS, which the suite cannot speak:_h2_client.inc:60openstcp://and nothing here talks h2 over TLS frame by frame. Even then a reactor tick between the microtask and the handler coroutine can flush the parked tail and close the window, so the phpt would be probabilistic.So the invariant is tested instead of the wire. The mode choice moves out of
h2_session_emit_exintoh2_tls_emit_use_drain, declared ininclude/http2/http2_session.hand compiled with or without OpenSSL so the unit suite links it. Four cases cover the selector; removing the pin turns exactly one of them red,test_emit_selector_parked_slice_outranks_the_mode. What this does not cover is the truncated frame on the wire, and the CHANGELOG says so.Verified
ctest16 of 16. Server suite 351 of 371 phpt, 20 skipped, 0 failed.