Skip to content

test/sample: add integration tests for streaming Bernoulli URL path#3775

Merged
jqnatividad merged 4 commits intomasterfrom
sample-streaming-tests
Apr 28, 2026
Merged

test/sample: add integration tests for streaming Bernoulli URL path#3775
jqnatividad merged 4 commits intomasterfrom
sample-streaming-tests

Conversation

@jqnatividad
Copy link
Copy Markdown
Collaborator

Summary

Closes the test-coverage gap flagged on #3774 (the Copilot review's last finding, which we deferred at merge time).

Stands up a local actix-web fixture server (port 8082, distinct from test_fetch's 8081) and adds four integration tests exercising the boundary detection and validation guards landed in #3774:

Test What it asserts
sample_bernoulli_url_quoted_newline_header Header field 0 contains a literal \n inside an RFC-4180 quote. The header arrives intact (3 fields, embedded newline preserved) and every emitted data row also has 3 fields. The pre-#3774 splitter would have split on the raw byte and corrupted every following record.
sample_bernoulli_url_max_size_truncation Serves a ~1.2 MiB CSV with fixed 100-byte records so --max-size 1 cuts deterministically inside record 10486 (cap is 1 MiB = 1 048 576 bytes; record 10486 ends at byte 1 048 611). Asserts max(id) <= 10485 (no half-record at the cap) and every emitted row is well-formed (5-digit id + 93-char 'X' payload).
sample_bernoulli_url_404_fails_fast Hits an unmapped path; actix-web returns 404. Asserts qsv exits non-zero — regression for the missing .error_for_status() chain we added to client.get(uri).send().await?.
sample_bernoulli_url_custom_delimiter Serves a TSV and passes --delimiter '\t'. Reads raw stdout and splits on tab (the writer also honors --delimiter, so the standard CSV read_stdout helper would collapse the row into a single comma-field). Asserts header and data rows both split into 3 fields.

All four use #[serial] so they share the port without racing.

Test plan

  • cargo build --locked --tests -F all_features — clean
  • cargo t sample -F all_features77/77 pass (was 73; +4 new)
  • cargo t sample_bernoulli_url -F all_features — 4/4 new tests pass
  • cargo clippy --locked --bin qsv -F all_features -- -D warnings — clean

🤖 Generated with Claude Code

Closes the test-coverage gap flagged in PR #3774. Stands up a local
actix-web fixture (port 8082, distinct from test_fetch's 8081) and
exercises the boundary detection and validation guards added there:

- sample_bernoulli_url_quoted_newline_header: header field 0 contains a
  literal `\n` inside an RFC-4180 quote. Asserts the header arrives
  intact (3 fields, embedded newline preserved) and that every emitted
  data row also has 3 fields. Old code would have split on the raw
  byte and corrupted every following record.
- sample_bernoulli_url_max_size_truncation: serves a ~1.2 MiB CSV with
  fixed 100-byte records so `--max-size 1` cuts deterministically
  inside record 10486. Asserts max id <= 10485 (no half-record at the
  cap) and that every emitted row is well-formed.
- sample_bernoulli_url_404_fails_fast: hits an unmapped path on the
  fixture server. Asserts qsv exits with error instead of feeding the
  HTML 404 body into the csv parser (regression for the missing
  `error_for_status()`).
- sample_bernoulli_url_custom_delimiter: serves a TSV and passes
  `--delimiter '\t'`. Reads raw stdout and splits on tab (the writer
  also honors --delimiter, so read_stdout's comma parser would
  collapse rows). Asserts header and data rows split into 3 fields.

Tests use #[serial] so they don't race on the port. 77/77 sample tests
pass; clippy --bin qsv clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 28, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Comment thread tests/test_sample.rs Fixed
Comment thread tests/test_sample.rs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

- (#1 #2) Replace SAMPLE_TEST_PORT + SAMPLE_TEST_HOST (which
  duplicated the port and could drift) with a single SAMPLE_TEST_PORT +
  SAMPLE_TEST_BIND_HOST literal. URL-builder and bind() both derive from
  the same source — no more brittle .split(':').next().unwrap() that
  would also panic on IPv6 hosts.
- (#3) Wrap the ServerHandle in a SampleWebServer RAII guard. The
  server now stops in Drop, so a panic inside read_stdout / stdout
  doesn't leak the port and cascade into "Address already in use" on
  the next #[serial] test.
- (#4) Call wrk.assert_success(&mut cmd) before reading stdout in
  the success-path tests, so a regression that makes qsv exit non-zero
  surfaces qsv's stderr instead of a generic CSV-parse error.

77/77 sample tests pass; clippy --bin qsv clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread tests/test_sample.rs Dismissed
Comment thread tests/test_sample.rs Dismissed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comment thread tests/test_sample.rs Outdated
Comment thread tests/test_sample.rs
Comment thread tests/test_sample.rs Outdated
Comment thread tests/test_sample.rs Outdated
…ver start timeout

- Replace the assert_success-then-read_stdout double-run pattern with a
  single capture-and-parse helper. The previous shape ran qsv twice per
  test, doubling fixture-server requests (and the ~1.2 MiB max-size
  download) and meaning the parsed stdout came from a different
  execution than the one whose status was asserted.
  - Added run_and_assert_success(): runs once, asserts status, returns
    Output (with stderr surfaced on failure).
  - Added parse_csv_stdout(): mirrors wrk.read_stdout's Vec<Vec<String>>
    shape but reads from a captured buffer.
  - All three success-path tests (quoted newline header, max-size
    truncation, custom delimiter) now use these helpers.
- Switch the SampleWebServer startup channel to send
  Result<ServerHandle, String> and use recv_timeout(10s) instead of
  recv(). A failed bind (e.g., port already in use) used to leave
  start() blocked forever; it now panics fast with the bind error
  surfaced from the server thread.

77/77 sample tests pass; clippy --bin qsv clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jqnatividad jqnatividad merged commit d3682d7 into master Apr 28, 2026
14 of 16 checks passed
@jqnatividad jqnatividad deleted the sample-streaming-tests branch April 28, 2026 15:46
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.

3 participants