Skip to content

Fix missing benchmark comparison in GitHub step summary#1154

Merged
NovemberZulu merged 1 commit into
amazon-ion:masterfrom
NovemberZulu:show-regression
Jun 22, 2026
Merged

Fix missing benchmark comparison in GitHub step summary#1154
NovemberZulu merged 1 commit into
amazon-ion:masterfrom
NovemberZulu:show-regression

Conversation

@NovemberZulu

Copy link
Copy Markdown
Collaborator

Description of changes:

Currently Detect regression step fail silently, see, e.g., #1151 , #1152, #1153. This happens because:

  1. GitHub sets +e mode by default
  2. Comparison apparently exists with non-zero error code of failure
  3. This error code is propagated to the whole a=$(...) line
  4. Shell exists at this point and if [ ! -z "$a" ]; then echo "${a}" >> $GITHUB_STEP_SUMMARY; exit 1; fi line isn't executed.

This PR is a simple fix that ensure that a=$(...) always exits with zero code.

Basic verification of bash behavior`

❯ bash
$ a=$(echo val1 && false)
$ echo $?
1
$ a=$(echo val2 && false || true)
$ echo $?
0
$ echo $a
val2

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.97%. Comparing base (3c1b6b1) to head (ee60d96).
⚠️ Report is 148 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1154      +/-   ##
============================================
+ Coverage     67.23%   67.97%   +0.74%     
- Complexity     5484     5624     +140     
============================================
  Files           159      160       +1     
  Lines         23025    23288     +263     
  Branches       4126     4176      +50     
============================================
+ Hits          15481    15831     +350     
+ Misses         6262     6164      -98     
- Partials       1282     1293      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NovemberZulu NovemberZulu merged commit 7159685 into amazon-ion:master Jun 22, 2026
10 checks passed
@NovemberZulu NovemberZulu deleted the show-regression branch June 23, 2026 16:36
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