fix(cube-bench): fix warmup display error

[luzhixing12345]

fix cube-bench warmup display error

./bin/cube-bench -c 10 -n 200 -w 3

before:

after:

[cubesandboxbot]

Review: fix(cube-bench): fix warmup display error (#1306)

Verdict: Approve — the fix is correct and well-scoped; only minor, non-blocking observations.

What the change does

Previously, warmup ran inside the RunBenchmark goroutine, which was launched concurrently with the Bubble Tea TUI (go RunBenchmark(cfg, resultCh) right before p.Run()). Warmup's fmt.Printf lines therefore interleaved with the TUI's terminal rendering, garbling the display. The PR:

1. Extracts a synchronous RunWarmup(cfg, out) that runs before the TUI starts, writing warmup progress to stdout cleanly.
2. Factors the http.Client construction into newHTTPClient(concurrency), and passes the warmup-created client into RunBenchmark(cfg, resultCh, client) so the connection pool is still shared between warmup and measurement.
3. Keeps a if !cfg.DryRun && client == nil fallback in RunBenchmark, preserving the no-warmup (-w 0) and dry-run paths.
4. Adds TestRunWarmupCompletesBeforeBenchmark covering the happy path.

The refactor is idiomatic, io.Writer is a nice touch for testability, and the nil client never gets dereferenced on the dry-run path (benchOneDry doesn't use it). I found no correctness bugs.

Observations

1. Report semantics change — total_time_s / throughput_qps now exclude warmup (should be documented).
Placing RunWarmup before startTime := time.Now() means cfg.elapsed — and with it total_time_s/throughput_qps in both the terminal summary (report.go) and the exported JSON — no longer includes warmup wall time, whereas the old code counted it (warmup ran inside the goroutine started after startTime). This is arguably more correct (the TUI's elapsed already excluded warmup, so the final report is now consistent with what the TUI displayed), and it's called out in the RunWarmup doc comment as intentional. But it is a behavior change beyond the stated "display fix" and isn't mentioned in the PR description. Note that tests/perf/cubebench.sh:835-836 still documents the old semantics ("cube-bench folds the warmup wall time into total_time_s / throughput_qps") as a reason the perf suite avoids -w; that comment is now stale. No functional impact on the perf suite (it runs with -w 0), but the comment should be updated.

2. Test coverage gap (minor).
TestRunWarmupCompletesBeforeBenchmark covers the happy path well but not the branches that guard the nil client handed to the benchmark loop:

• RunWarmup returning nil when cfg.Warmup == 0 (non-dry-run) — this is the exact path that exercises the client == nil fallback in RunBenchmark, which is itself untested.
• RunWarmup returning nil when cfg.DryRun is true.

3. Trivial nit.
RunWarmup guards on cfg.Warmup == 0 rather than <= 0, so a nonsensical negative -w value would still emit the trailing blank line (fmt.Fprintln(out)); the old if cfg.Warmup > 0 guard did not. Not a realistic input — ignore or switch to <= 0 for consistency.

AI-generated review; not a human approval.

[luzhixing12345]

also fixed qps calculate bug

[ls-ggg]

LGTM