Skip to content

benchmark.sh: apply PR #202 review-thread fixes for parsing, warnings, and portability#203

Closed
Copilot wants to merge 2 commits into
developfrom
copilot/fix-code-for-comments
Closed

benchmark.sh: apply PR #202 review-thread fixes for parsing, warnings, and portability#203
Copilot wants to merge 2 commits into
developfrom
copilot/fix-code-for-comments

Conversation

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

This updates benchmark.sh to resolve all comments in the referenced PR review thread, applying the requested behavior changes directly. The fixes are scoped to portability, timing-value parsing, warning criteria, and summary wording.

  • Portable temp file creation

    • Replaced bare mktemp with a template-backed invocation so the script works on macOS/BSD and Linux variants.
  • Timing parse normalization (zero0)

    • Updated parse_dtest_output to normalize dtest’s literal zero timing output to numeric 0 for parsed user/sys/avg fields.
  • Warning signal narrowed to required fields

    • Changed incomplete-output warning logic to trigger only when required timing fields (user, sys) are missing, avoiding false warnings when optional fields are absent.
  • Summary wording aligned with actual computation

    • Updated compare-summary label from “total user ms” to “avg user ms” to match the existing average-based calculation.
# before
RESULTS="$(mktemp)"
if [[ "$parsed" == *"NA"* ]]; then ...

# after
RESULTS="$(mktemp "${TMPDIR:-/tmp}/dds-benchmark.XXXXXX")"
read -r parsed_user parsed_sys _ _ <<<"$parsed"
if [[ "$parsed_user" == "NA" || "$parsed_sys" == "NA" ]]; then ...

Copilot AI changed the title [WIP] Fix code based on review comments benchmark.sh: apply PR #202 review-thread fixes for parsing, warnings, and portability Jun 19, 2026
Copilot AI requested a review from tameware June 19, 2026 18:35
@tameware tameware closed this Jun 21, 2026
@tameware tameware deleted the copilot/fix-code-for-comments branch June 21, 2026 19:02
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