Skip to content

test: drain loopback HTTP request bodies - #260

Merged
smiggleworth merged 2 commits into
mainfrom
fix/windows-http-fixture-request-drain
Aug 25, 2026
Merged

test: drain loopback HTTP request bodies#260
smiggleworth merged 2 commits into
mainfrom
fix/windows-http-fixture-request-drain

Conversation

@smiggleworth

@smiggleworth smiggleworth commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • normalize accepted loopback sockets to blocking mode on every platform
  • drain loopback HTTP requests through their declared Content-Length before sending scripted responses
  • share the complete-request reader between scripted and recording provider fixtures
  • keep fixture reads bounded while allowing the full client request to arrive

Why are you making this contribution?

The post-merge Windows Platform run for PR #259 failed should_retry_s3_conditional_request_conflict_when_creating_object. The scripted server performed one arbitrary 4 KiB read, replied, and closed without proving the PUT body had been consumed. A first refinement exposed the platform-specific root cause directly: accepted sockets inherited the listener's nonblocking mode on Windows, so the read returned WouldBlock. Some fixtures panicked; others ignored the error, closed with the request unread, and surfaced a reset instead of their scripted response.

Linked issues

No issue: follow-up to the Windows Platform failure found while completing PR #259.

Acceptance audit

  • Criterion: Loopback HTTP fixtures read accepted connections consistently on Windows, macOS, and Linux.
    Evidence: Platform run 32861046364 captured Windows error 10035 (WouldBlock) from the inherited nonblocking accepted socket before the refinement; Platform run 32862582023 passes the complete workspace on both Windows and macOS after it.
    Production entry point: The regressions exercise CloudExecutor::execute_request and S3Backend::submit_put through real loopback HTTP connections; only test support is modified.
    Resolution: Explicitly switches every accepted connection from a nonblocking listener back to blocking mode with a bounded read timeout.
  • Criterion: Provider HTTP fixtures consume complete fixed-length requests before closing their response connection.
    Evidence: storage::providers::s3::tests::should_retry_s3_conditional_request_conflict_when_creating_object passes after the fixture change; Platform run 32859180767 captured the initial Windows failure.
    Production entry point: The regression exercises S3Backend::submit_put and the real cloud executor through a loopback HTTP connection.
    Resolution: Reads headers, parses Content-Length case-insensitively, and continues reading until the declared body is present before writing the scripted response.
  • Criterion: Recording fixtures retain their existing request metadata behavior while avoiding the same unread-body close pattern.
    Evidence: cargo test --workspace --all-features passes the complete library, integration, and doctest suite, including provider header and request-shape tests.
    Production entry point: AWS, Azure, and GCS tests exercise their production provider implementations against the shared recording fixture.
    Resolution: Reuses the complete-request reader before parsing the existing method, target, and headers.

Risk and compatibility

Low risk and test-only. No public API, production behavior, persistence format, dependency, or runtime timeout changes. Fixture reads retain a five-second socket timeout and caller-owned server shutdown.

Verification

  • cargo test --all-features storage::providers::s3::tests::should_retry_s3_conditional_request_conflict_when_creating_object -- --exact --nocapture — passed
  • cargo test --lib --all-features storage::cloud::executor::tests::should_not_retry -- --nocapture — passed; all three mutation retry tests
  • cargo test --workspace --all-features — passed; complete library, integration, and doctest suite
  • cargo clippy --workspace --all-targets --all-features -- -D warnings -D clippy::pedantic — passed
  • cargo fmt --check — passed
  • cntryl-tools validate-tests — passed; 2,440/2,440 compliant
  • git diff --check — passed
  • Platform run 32862582023 — passed; Windows and macOS release builds and complete workspace tests

Tool assistance disclosure

  • AI or another generative tool materially assisted this contribution.

Codex correlated the Windows socket failures with inherited nonblocking accepted sockets and incomplete request reads, refined both fixture families, reviewed the diff, and ran the verification above.

Contributor responsibility

  • I understand the complete change and can explain or revise it.
  • I reviewed the complete diff.
  • I reported validation accurately and did not claim checks I did not run.
  • I disclosed material generated assistance.
  • I have the right to submit this work under the repository's license.

Review checklist

  • Behavioral coverage uses the production S3 provider path, not only the private fixture helper.
  • The change is confined to test support.
  • Request reads and fixture shutdown remain bounded.

Copilot AI lite review requested due to automatic review settings August 25, 2026 14:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@smiggleworth
smiggleworth merged commit e04ecb4 into main Aug 25, 2026
9 of 10 checks passed
@smiggleworth
smiggleworth deleted the fix/windows-http-fixture-request-drain branch August 25, 2026 15:17
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.

2 participants