Repository navigation
Phase 8a: synchronous transport — tests, the wire fixtures and doubles - #91
Merged
Wahbeh-Mohammad merged 15 commits intoSep 21, 2026
Merged
Wahbeh-Mohammad merged 15 commits into
Wahbeh-Mohammad merged 15 commits into
Conversation
…se 8a) dexpace-transport-net_http: Dexpace::Transport::NetHTTP with .build over a fresh-per-call Net::HTTP and .using over a caller's own, the Adapter, the per-response ResponsePump with its head-adaptation handshake, RequestMapper and ResponseMapper, Deadline, Failures, TLSSettings, ProxyRoute, the require-time registration under :net_http, and the gemspec's net-http >= 0.4. dexpace-conformance: the assertion protocol phase 0 postponed (Failure, Vacuous, Assertion, Result, Report), the twenty-eight-assertion TransportSuite, TransportCase with its SettleOnly guard, BorrowedPair, the WireServer fixture and its Scripts, MinitestDriver, the opt-in RSpecDriver, and the RecordingSpan and Allocations doubles. dexpace-core: Dexpace::TransportError < ::IOError, Configuration::Keys::REQUEST_TIMEOUT and Instrumentation::Events::TRANSPORT_HEADER_DROPPED, with the pins the two constants moved. Repository: tempfile on the require allowlist with its fixture, BUNDLE_PATH scoped in the clean-bundle gate, socket and tempfile on the conformance Steep target, the surface manifests regenerated once, the net-http Timeout-thread warm-up both adapter gems' test helpers require, and the bare-require helper core's repaired seam-surface test needs.
Phase 8a, review round 0's three code-branch nits. R0-6: the adapter's
ResponseMapper::LENGTH and the conformance fixture's RequestReader length
check were regexp literals a wire value reaches; both are now
::Regexp.new('\A[0-9]{1,15}\z', timeout: 1.0).freeze, core's spelling for
every such pattern (PacingParsers, P6-61), so no header hands #to_i an
unbounded digit run -- a sixteen-digit Content-Length is the unknown-length
sentinel like any other value the grammar refuses. R0-7: the rewritten swap
pin in core's transport_test.rb compared a Proc with an Array and could not
fail; it now snapshots Transport.registered_keys before the swap and asserts
the same list afterwards. R0-10: Allocations.delta re-enabled the collector
unconditionally in its ensure; it now records what GC.disable answered and
re-enables only when it found the collector running, so a host that had GC
off around the call finds it off afterwards. The request_reader sig gains
the private LENGTH declaration the Steep target needs.
Cancellation::Source runs an already-cancelled hook inline, so the
pump's own on_cancel { close } could run #release from inside the
constructor before @subscription was assigned, and #release then
raised NoMethodError on nil.detach after joining a producer that had
already put the request on the wire (review round 1, R1-2). On the
borrowing construction that raise reached Adapter#dispatch's rescue,
which pushed the permit a second time onto a full SizedQueue.
ResponsePump#initialize now asks the token first: a cancelled token
gets a closed pump with no thread started, no socket opened and the
borrowed permit handed straight back, exactly once. Otherwise the
thread is started and the subscription registered after it, as before,
with both slots initialised to nil so the inline #release a cancel
racing the registration triggers finds them; #release joins and
detaches nil-safely, and #produce reads the latch before it exchanges,
so a pump closed before its producer was scheduled never touches the
socket. The RBS types both ivars optional and declares the new private
start_producer. Recorded as P8-64.
Net::HTTP.new's default p_addr is :ENV, which consults URI#find_proxy on #start, and uri's find_proxy warns "The environment variable HTTP_PROXY is discouraged" whenever HTTP_PROXY is set and the lower-case http_proxy is not -- before its loopback exemption and before NO_PROXY is read (uri 0.12.5 through 1.1.1, every row). DexpaceTestCase makes every warning fatal, so on such a host test/support/net_http_warmup.rb, required by both adapter gems' test_helper, aborted every suite of both gems and rake test:gems at load (review round 2's R2-1, measured on 4.0.6 under HTTP_PROXY=http://127.0.0.1:9 with no http_proxy). Build the warm-up client the way the adapter builds its own through ProxyRoute: an explicit nil for the four proxy positionals. The comment records the mechanism and the rule the tests branch applies to every raw Net::HTTP a suite starts, so the suites are hermetic under HTTP_PROXY, http_proxy, HTTPS_PROXY and NO_PROXY alike; the two R17 controls that reach :ENV on purpose set http_proxy themselves.
ResponseMapper's YARD already says what an HTTP/1.0 head does and whose gap that is. Say the same for the status: phase 1's Status guards 100-599 (HTTP-10's reading), Net::HTTP parses any three digits and delivers a 999 or a 600 as an HTTPUnknownResponse, and such a head raises Dexpace::InvalidArgumentError after the head with the connection released (measured 2026-09-20; review round 2's R2-2). Whether TRANSPORT-24's "any code" reaches 600-999 is a phase-1 model question, routed to phase 10's inbound list on the docs branch. A comment only: no behaviour, gate or surface changes.
Review round 3's R3-1. ResponseMapper#parse_length! took any single,
grammar-matching Content-Length as the body's length, but net-http reads
a response carrying Transfer-Encoding: chunked under the chunked framing
whatever else the head says -- read_body_0 asks #chunked? before
#content_length on 0.4.1, 0.6.0 and 0.9.1 alike -- so the value was RFC
9112 section 6.3's overridden one and never the number of bytes the pump
would deliver. Every reader that copies exactly `content_length` bytes
(ResponseBody#each, #write_to, #to_replayable) therefore truncated a
chunked body silently when the header was short ("0123456789" delivered
as "01234" under Content-Length: 5) or raised StreamError.short_transfer
when it was long, while #body_string, which drains to end of stream,
read all of it. TRANSPORT-27 maps an invalid Content-Length to the
unknown-length sentinel, and a Content-Length the framing overrides is
the invalid case R4's grammar could not see.
The guard gains `!native.chunked?` -- Net::HTTPHeader#chunked?, the
predicate read_body itself consults -- so such a header is the -1
sentinel like any other value the mapper refuses: the pump reads to end
of stream and every reader delivers the whole body. The native delete in
that branch changes nothing read_body consults under a chunked encoding
and is kept for the one-branch shape. The wire value still reaches the
caller's Dexpace::Headers, as before. R4's comment states the case.
Measured through the real adapter against WireServer on 4.0.6: both
directions now answer -1 and deliver ten bytes through #each, #write_to
and #to_replayable.
…conformance gem The adapter's ten suites (adapter, response pump, request and response mappers, deadline, failures, TLS settings, proxy route, the conformance driver against the real adapter, the generator slice with its guarded codec half, and the matrix facts printing each row's active net-http version) with AdapterFixtures and NetHTTPRecordingSink; the conformance gem's nineteen suites with the RawWireTransport double and its twenty named defects, NonConformingTransport, StubTransport and AssertionProbe; core's TransportError suite and the bare-require registry tests; and the allowlist gate test's comment naming tempfile.
Phase 8a, review round 0. R0-1: the TRANSPORT-22 adaptation-failure fixture declares Content-Length: 100, sends two bytes and holds the connection, so only Adapter#dispatch's close can release it -- mutation H (the guard removed) now fails the bounded await and strands the producer the test base's thread count catches. R0-2: the two clamp assertions in deadline_test.rb carry an explicit 0.0 delta, since Minitest's default 0.001 is MIN_TIMEOUT_SECONDS exactly and swallowed mutation N. R0-3: the subscription test reads the Cancellation::Source's hook list, one longer while the pump is open and back to its size after the close, the way core's own cancellation suite observes a bounded registration; mutation E (the detach removed) now fails. R0-8: the plain-http TLS test's comment and name say what it proves -- a tls: hash is inert on a plaintext call -- and record mutation X as unobservable. R0-9: the origin-401 test configures a credentialled proxy through the chain and lets the fixture play it: one request carrying the preemptive Basic, a 401, and no second request. Three guards for the code branch's round-1 lines: the sixteen-digit Content-Length that ResponseMapper's bounded grammar maps to the sentinel and RequestReader's frames as no body, each grammar's per-pattern timeout, and a collector the host had disabled staying disabled after Allocations.delta. Each was run red against its reverse mutation.
Phase 8a, review round 1. The suite's header said every configuration is a hermetic from_hash source; since round 1 the origin-401 test drives the whole adapter through Dexpace.configure's override tier -- above the environment -- and resets it in an ensure, and the header now says so.
Review round 1's R1-1: the two tests that drive the whole adapter through Dexpace.configure overrode HTTP_PROXY alone over Configuration::EMPTY, whose environment tier is the real ENV, so a host HTTPS_PROXY won the resolver's preference and a host NO_PROXY covering the target bypassed the fixture. Both now override all three keys the resolver reads, and the origin-401 test's comment gives the real reason for its TEST-NET-1 target. Measured while fixing it: with HTTPS_PROXY=http://127.0.0.1:9 exported, fifteen of the adapter suite's thirty-four tests routed their loopback fixture through that proxy, because every owning adapter resolves its proxy through the process-wide configuration. NetHTTPHermeticProxy blanks the three keys through the override tier around every test that includes AdapterFixtures and around the conformance driver, never touching ENV, and the two Net::HTTP :ENV controls clear no_proxy for the library's own find_proxy. The gem's suite is green under HTTPS_PROXY=http://127.0.0.1:9 NO_PROXY=192.0.2.1 and under a clean environment alike. R1-2: two tests in a fourth nested class of the pump suite build a pump over an already-cancelled source, against a fixture that holds before headers: the pump comes back closed, its first read is the cancellation, no connection reached the fixture and, on the borrowing construction, the permit is back exactly once. Both are red against the unfixed pump (NoMethodError on nil.detach plus a leaked producer) and against the nil-safe release alone (one connection, a producer outliving the join, the permit held). The bounded-join source scan admits the nil-safe spelling and still refuses an unbounded join.
With the pump refusing a cancelled token on its own (P8-64), the adapter suite's "already-cancelled token is refused before anything reaches the wire" case no longer told Adapter#perform's cancellation.check! from the pump's: removing the check left the same CancelledError and the same zero connections. P8-52's claim is that the check runs before anything is MAPPED, so the case now sends a request carrying a managed header through an adapter with a recording sink and asserts that no TRANSPORT_HEADER_DROPPED event was logged. Mutation B is red again: the Expect drop is logged when the check is gone.
Review round 2 measured that under an upper-case HTTP_PROXY with no lower-case http_proxy, every raw Net::HTTP the suites start through Net::HTTP.new's :ENV default hit uri's "HTTP_PROXY is discouraged" warning, which the test base makes fatal: 3 failures and 17 errors in the adapter gem and 6 and 14 in the conformance gem once the warm-up alone was fixed. Pass the four proxy positionals as nil, exactly as the adapter's ProxyRoute does, in AdapterFixtures#client_for, the conformance driver's borrow factory, and the Net::HTTP.start calls in wire_server_test.rb and scripts_test.rb. The two R17 controls keep :ENV on purpose and set the lower-case name themselves. Two guards make the property host-independent: adapter_test.rb's ProxyTest drives a client_for client through the borrowed adapter with HTTP_PROXY exported and http_proxy cleared, and wire_server_test.rb's new PlainClientTest does the same through Fetch#get. A fixture reverted to :ENV turns each red with the warning itself, whatever the host's environment says.
NetHTTPHermeticProxy blanks the SDK's three configuration keys and nothing else; the library's own :ENV default is the other reader of the environment, and the fixtures keep off it by passing an explicit nil proxy. The module's comment now names both halves, the uri warning that makes the second one necessary, the fixture sites that honour it, and the two guards that hold it on any host (review round 2's R2-1).
…join
Review round 3's R3-1, R3-2 and R3-3, on the tests layer.
R3-1: response_mapper_test.rb's BodyTest gains a case over a native
head carrying both Transfer-Encoding: chunked and a Content-Length of 5,
asserting the -1 sentinel, the raw header in the caller's Headers and
the whole ten-byte body through #body_string; a coding LIST ("gzip,
chunked") answers -1 too, because the predicate is net-http's own
#chunked?. adapter_test.rb's WireTest drives the same head through the
real adapter against WireServer in both directions -- a short header
through #each, which truncated to "01234", and a long one through
#to_replayable, which raised StreamError.short_transfer -- and asserts
the whole body from each. With the code fix reverted the unit case
fails on the sentinel and the wire case errors with the StreamError.
R3-2: response_pump_test.rb's TeardownTest gains the two cases the
closed-pump guard at the top of #readpartial lacked: a one-byte read
leaves residue in the pump, then a close (resp. a cancel through the
token) and the next one-byte read raises ClosedError (resp.
CancelledError with the reason) instead of serving the residue. Every
earlier test drained with a large maxlen or through #body_string, where
the residue is empty and #fill's own check fires; with the #readpartial
guard removed both new cases fail with "nothing was raised".
R3-3: wire_server_test.rb's "promptly" test takes its thread baseline
BEFORE WireServer.start and asserts the count is back to it after
#close (both threads gone, not merely one), with the two-threads-alive
count asserted before the close; and a source-scan case, the shape the
pump's JOIN_DEADLINE test uses, asserts the accept-thread join, the
per-handler join and that no join in the fixture is unbounded -- on MRI
IO#close on the accepted socket usually lets the handler finish before
#close returns, so the count alone tells the join from its absence only
when the handler loses that race (three of five runs here); the scan
tells them apart every time.
… tree The slice test's third case names phase 7a's Dexpace::Serde::JSON::Codec, which 8a's base did not carry, so it was written under a `skip` guard that could never run. Reconciling the 8a stack onto the tree that holds the whole of phase 7 removes the guard and runs the case for real. Two repairs by the minimum, both to the test alone: an explicit `require "dexpace/serde/json"` beside the existing cross-gem require of the conformance gem, because nothing else on the adapter's load path defines the codec; and the three lines that read the mapped error's headers and body, written against 4b's design names (`#headers`, `#body`, `#read_fully`), now read `#response.headers` and `#response.body_string` -- the as-built ProtocolError carries the buffered response, and body_string over BODY-30's BufferBody is what makes "readable twice after the socket is gone" a real assertion. The suite's skip count drops from two to one: the remaining skip is conformance_test.rb's TRANSPORT-18 vacuity, reported as a Minitest skip by design §9.3.
Wahbeh-Mohammad
added
type:feature
Wahbeh-Mohammad
changed the base branch from
30-phase-8a-synchronous-transport-and-conformance
to
main
September 21, 2026 07:28
Wahbeh-Mohammad
deleted the
30-phase-8a-synchronous-transport-and-conformance-tests
branch
September 21, 2026 07:30
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
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.
Part of #30. Second PR of phase 8a's stack — the tests — on top of #90.
What lands
40 files, +4,905: eleven
net_httpsuites, nineteenconformancesuites, two core suites, six doubles.gems/dexpace-transport-net_http/test/dexpace/transport/net_http/:adapter_test.rb,conformance_test.rb(the shared suite driven against the real adapter — one vacuous row,TRANSPORT-18, reported as a skip),deadline_test.rb(theMIN_TIMEOUT_SECONDSclamp with an explicit0.0delta — round 0's R0-2, the default delta had equalled the clamp),failures_test.rb,generator_slice_test.rb(the socket half of the generator path throughOperation→Pipeline.standard→ the adapter → the wire →TypedResponse; its codec half, guarded on the old base, runs for real against 7a's codec since the rebase),matrix_facts_test.rb(twelve facts printing the activeNet::HTTP::VERSIONper row),proxy_route_test.rb(hermetic:HTTP_PROXY,HTTPS_PROXYandNO_PROXYall overridden in the same block, round 1's R1-1; the 401 clause against a real proxy fixture, R0-9),request_mapper_test.rb,response_mapper_test.rb(the chunked-with-Content-Lengthcase, round 4),response_pump_test.rb(the subscription detached — R0-3 through theCancellation::Source's hook count; the pump built over an already-cancelled token — R1-2; a closed or cancelled pump refusing a small read even with residue buffered — R3-2),tls_settings_test.rb.gems/dexpace-conformance/test/dexpace/conformance/: nineteen suites includingtransport_suite/{inbound,lifecycle,outbound,resilience,streaming}_test.rb(every assertion driven againstStubTransport/RawWireTransport— whoseleave_opendefect keeps its socket referenced so a GC cannot make it pass — andNonConformingTransport) andwire_server_test.rb(the handler join with the thread baseline taken beforeWireServer.start— R3-3).error/transport_error_test.rb,transport_bare_require_test.rb.AdapterFixtures,NetHTTPRecordingSink(renamed fromRecordingSinkafter the one-processtest:gemsrun exposed the collision with core's double),AssertionProbe,NonConformingTransport,RawWireTransport,StubTransport.Verification
Tests tip (rebased onto phase 7's
main):bundle exec rake(all eighteen) green on 4.0.6 — 3,698 runs / 72,656 assertions / 0 failures / 1 skip (TRANSPORT-18's vacuity, by design) / 99.88%; the matrix set green on 3.2.11 under net-http 0.4.1 and 0.9.1, 3.3.12 and 3.4.10; honest RuboCop clean; nine socket and timing suites ten times undertimeout 120; 41 test files alone under-wwith zero warnings. Forty-seven guards run red and recorded in the checklist; the reviewers' 50 + 48 + 55 + 52 + 39 mutations.Known follow-ups
R0-5 and the recorded equivalent mutants — see the code PR.