Removed dtest --min/--max flags.

[tameware]

--min/--max reported minimum and maximum batch averages, not single-deal times, so they were useless.

Use -r for per-board outliers instead.

[Copilot]

Pull request overview

This PR removes the dtest --min/--max CLI flags and the underlying “min/max per-hand across batches” tracking in TestTimer, since those extremes were based on batch averages and weren’t useful for identifying per-board outliers (which are now intended to be handled via -r/--report).

Changes:

• Deleted TestTimer batch min/max tracking state and accessors; simplified print_hands() to always print the same summary.
• Removed --min/--max from CLI option parsing (args.cpp), options struct (cst.hpp), usage docs, and updated call sites.
• Updated unit tests to validate accumulation/printing behavior and to ensure former flags are not accepted.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
library/tests/TestTimer.hpp	Removes min/max-related members/APIs and simplifies print_hands() signature/docs.
library/tests/TestTimer.cpp	Drops min/max tracking logic and conditional printing; removes now-unused include.
library/tests/testcommon.cpp	Updates call to print_hands() to match new signature.
library/tests/test_timer_test.cpp	Reworks tests to validate accumulation/printing instead of min/max extremes.
library/tests/cst.hpp	Removes show_min_ / show_max_ options fields.
library/tests/args.hpp	Updates documentation to remove mention of min/max timing summary.
library/tests/args.cpp	Removes --min/--max options from CLI parsing and usage output.
library/tests/args_test.cpp	Removes old min/max option tests and adds rejection coverage for former flags.

[tameware]

Addressed Copilot's note in fb39676: UnknownMinMaxFlagsAreRejected now asserts both --min and --max are rejected.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

library/tests/test_timer_test.cpp:129

• In ResetClearsAccumulatedStats, EXPECT_NE(out.find("0"), ...) is too permissive: it would still pass if the hands count were 10, since the output contains the substring "0". Make the assertion specific to the "Number of hands" line showing exactly 0 so the test reliably detects a non-reset count_.

  const std::string out = capture_print_hands(timer);
  EXPECT_NE(out.find("Number of hands"), std::string::npos);
  EXPECT_NE(out.find("0"), std::string::npos);
  EXPECT_EQ(out.find("User time (ms)"), std::string::npos);

[tameware]

Addressed Copilot's suppressed note in ff0023f: ResetClearsAccumulatedStats now requires the Number of hands field to be exactly 0 (not a substring match).

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

library/tests/test_timer_test.cpp:117

• Similar to the previous test, these expectations rely on substring matches for "100" and "10.00" rather than verifying the specific fields. Anchoring to the output labels avoids accidental matches and makes the test intent clearer.

  EXPECT_NE(out.find("Number of hands"), std::string::npos);
  EXPECT_NE(out.find("100"), std::string::npos);
  EXPECT_NE(out.find("10.00"), std::string::npos);

library/tests/test_timer_test.cpp:102

• These assertions use substring searches for numeric values (e.g. "15", "120"), which can produce false positives if those digits appear elsewhere in the formatted output. Using regex anchored to the field labels makes the test validate the intended lines and is more robust to formatting changes.

This issue also appears on line 115 of the same file.

  EXPECT_NE(out.find("Number of hands"), std::string::npos);
  EXPECT_NE(out.find("15"), std::string::npos);   // 10 + 5 hands
  EXPECT_NE(out.find("120"), std::string::npos);  // 100 + 20 user ms
  EXPECT_NE(out.find("8.00"), std::string::npos); // avg user 120/15

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

library/tests/TestTimer.hpp:80

• Grammar: “Non-positive hands is ignored” should be plural (“hands are ignored”).

    /// Non-positive hands is ignored (no cumulative updates).

library/tests/BUILD.bazel:63

• PR title/description focuses on removing dtest --min/--max, but this PR also changes Bazel test sizing (and removes a calc_par performance test). Consider updating the PR description/title to mention the MSAN timeout-driven sizing changes (or split into a separate PR) so reviewers/CI owners have the right context.

# Standalone calc_par API test.
# medium: MSAN can spend ~80s on the remaining suite alone (small = 120s under
# --config=msan), so keep headroom against runner load variance.
cc_test(
    name = "calc_par_test",

[tameware]

Grammar: “Non-positive hands is ignored” should be plural (“hands are ignored”).

No, it's correct as is.

PR title/description focuses on removing dtest --min/--max, but this PR also changes Bazel test sizing (and removes a calc_par performance test).

That's a cherry-pick of the orthogonal fix necessary to get the CI to pass. It will disappear in the merge.