google / google/capsem

[triage:tooling-05-missing-tests-summarize-aggregation] No test covers `summarize()` — the core aggregation that turns samples into the reported throughput/error schema

Open
#149 0 comments 0 reactions 0 assignees View on GitHub
type:bug
Dominant language
Rust
Stars
72
Forks
13
Avg merge
1d 2h
Merged PRs (30d)
5

Description

Imported from Capsem triage report `tooling-05-missing-tests-summarize-aggregation.md`.

- Severity: `medium`
- Category: `missing-test`
- Area: `capsem-bench`
- Location: `crates/capsem-bench/src/main.rs:798-853` (`summarize`); tests in `mod tests` at `1231-1552`
- Confidence: `verified`

## Summary
`summarize()` is the function that converts raw `RequestSample`s into the emitted `ScenarioResult` (successful/failed counts, the `errors` histogram, `transfer_bytes`, `requests_per_sec`, `bytes_per_sec`, and the secret-leak flags). It is the single most consequential measurement-correctness function in the crate, and it has zero direct test coverage. The existing tests exercise `select_scenarios`, `latency_summary`, `result_ok`, the DNS query builder, delta math, command construction, and shell quoting — but never call `summarize`.

## Evidence
Grep of the test module (`mod tests`, lines 1231-1552) shows no call to `summarize`. The tested helpers are `select_scenarios` (1237), `latency_summary` (1250), `result_ok` (1259), `build_dns_query`/`parse_dns_rcode` (1291), delta math via `rows_by_name`/`ratio` (1303), `validate_successful_scenarios` (1382), `guest_protocol_command` (1389), `parse_guest_protocol_artifact` (1413), `shell_quote` (1466), `build_delta_report` (1480). `summarize` aggregates all of those into the final artifact and is never invoked.

Specific untested behaviors that affect reported numbers:
- `failed = total_requests.saturating_sub(successful)` vs sample count (see tooling-03).
- The `else if` error-classification chain (lines 810-828): status mismatch vs `required_text_missing` vs `bytes:!=` are mutually exclusive per sample, so a sample failing two checks is only counted once — never asserted anywhere.
- `transfer_bytes` summation and `bytes_per_sec` rounding.
- `secret_shaped_fixture_seen` = `Some(true/false)` only for secret-shaped scenarios, `None` otherwise; `raw_secret_stored_in_result` always `Some(false)` for those scenarios.

## Impact
Regressions in the error histogram, byte accounting, or the secret-leak flags would ship silently. These numbers feed regression-detection diffs and the leak-detection invariant; an incorrect `successful`/`failed` split or a dropped error category would not be caught.

## Suggested fix
Add table-driven tests over `summarize` with hand-built `RequestSample` vectors: all-success, mixed status errors, byte-size mismatch, missing-required-text, an explicit `error`, and a secret-shaped scenario both seeing and not-seeing the marker. Assert `successful`, `failed`, the full `errors` map, `transfer_bytes`, and the two secret flags.

## Triage
Confirmed from the local reviewed report in `/Users/elie/git/capsem/tmp/bugs/tooling-05-missing-tests-summarize-aggregation.md`. Track implementation in the triage sprint; add regression coverage before fixing.

Contributor guide

Open the contributing guide

Research direction

Start in crates/capsem-bench/src/main.rs at summarize (lines 798-853), then read the existing mod tests (1231-1552) and the RequestSample and ScenarioResult types it uses. Add table-driven coverage for the listed success, error, byte-accounting, and secret-shaped cases, and run the capsem-bench tests. Done means the assertions cover counts, the full errors map, transfer_bytes, and both secret flags.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
performance, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.