Skip to content

fix: fix inconsistency in canbench summary report#107

Merged
maksymar merged 19 commits intomainfrom
maksym/summary-fix
May 13, 2025
Merged

fix: fix inconsistency in canbench summary report#107
maksymar merged 19 commits intomainfrom
maksym/summary-fix

Conversation

@maksymar
Copy link
Copy Markdown
Contributor

@maksymar maksymar commented May 12, 2025

This PR fixes #106 by moving results analysis fully into the canbench binary, removing it from the run script.

Changes:

  • Uploads CSV report as a job artifact and links it in the summary report
  • Adds explicit status into metric summary
  • Removes empty first line from the report output
  • Fills empty cells in the CSV report

image

Tested on stable-structures PR #301.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2025

canbench 🏋 (dir: examples/fibonacci) 3289196 2025-05-13 08:21:32 UTC

examples/fibonacci/canbench_results.yml is up to date
📦 canbench_results_benchmark-fibonacci-example.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes detected 👍
    counts:   [total 5 | new 0 | improved 0 | regressed 0 | unchanged 5]
    change:   [min 0 | med 0 | max 0]
    change %: [min 0.00% | med 0.00% | max 0.00%]

  heap_increase:
    status:   No significant changes detected 👍
    counts:   [total 5 | new 0 | improved 0 | regressed 0 | unchanged 5]
    change:   [min 0 | med 0 | max 0]
    change %: [min 0.00% | med 0.00% | max 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 5 | new 0 | improved 0 | regressed 0 | unchanged 5]
    change:   [min 0 | med 0 | max 0]
    change %: [min 0.00% | med 0.00% | max 0.00%]

---------------------------------------------------
Successfully saved CSV results to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2025

canbench 🏋 (dir: examples/btreemap_vs_hashmap) 3289196 2025-05-13 08:21:32 UTC

examples/btreemap_vs_hashmap/canbench_results.yml is up to date
📦 canbench_results_benchmark-btreemap-vs-hashmap-example.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes detected 👍
    counts:   [total 3 | new 0 | improved 0 | regressed 0 | unchanged 3]
    change:   [min 0 | med 0 | max 0]
    change %: [min 0.00% | med 0.00% | max 0.00%]

  heap_increase:
    status:   No significant changes detected 👍
    counts:   [total 3 | new 0 | improved 0 | regressed 0 | unchanged 3]
    change:   [min 0 | med 0 | max 0]
    change %: [min 0.00% | med 0.00% | max 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 3 | new 0 | improved 0 | regressed 0 | unchanged 3]
    change:   [min 0 | med 0 | max 0]
    change %: [min 0.00% | med 0.00% | max 0.00%]

---------------------------------------------------
Successfully saved CSV results to canbench_results.csv

@maksymar maksymar requested a review from Copilot May 12, 2025 14:36
Copy link
Copy Markdown

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

This PR fixes inconsistencies in the canbench summary report output by adding a descriptive "status" field and standardizing percentage change reporting across expected output files, as well as making minor improvements in production code. Key changes include:

  • Updating expected report files to include a new "status" line with descriptive text and replacing "n/a" with computed percentage changes.
  • Modifying the summary reporting in src/summary.rs to use status strings and infinity values in cases with zero baselines.
  • Renaming CSV headers in src/csv_file.rs and updating CI workflows to upload CSV artifacts.

Reviewed Changes

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

Show a summary per file
File Description
canbench-bin/tests/expected/*.txt Adjusted expected outputs to include a new "status" field and explicit change percentage values.
canbench-bin/src/summary.rs Updated status handling in the summary report and pushed infinity values when comparing against a zero baseline.
canbench-bin/src/lib.rs Adjusted newline placements when printing results to refine spacing.
canbench-bin/src/csv_file.rs Renamed CSV header fields and modified the percentage formatting function for clarity.
.github/workflows/ci.yml Added steps to upload CSV results artifacts.
Comments suppressed due to low confidence (1)

canbench-bin/src/lib.rs:123

  • [nitpick] Verify that the adjustment to newline placement in the show_results block still produces the intended output formatting.
if show_results { println!("---------------------------------------------------"); ... }

Comment thread canbench-bin/src/csv_file.rs Outdated
@maksymar maksymar marked this pull request as ready for review May 12, 2025 14:41
@maksymar maksymar requested a review from a team as a code owner May 12, 2025 14:41
Comment thread canbench-bin/src/summary.rs
@maksymar maksymar merged commit 5138e12 into main May 13, 2025
13 checks passed
@maksymar maksymar deleted the maksym/summary-fix branch May 13, 2025 08:23
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.

Inconsistent summary reporting between canbench binary and posting scripts

3 participants