Skip to content

Benchmark diffs not being output on threshold regression#3515

Merged
rnro merged 1 commit intoapple:mainfrom
rnro:benchmarks_script_missing_diffs
Feb 20, 2026
Merged

Benchmark diffs not being output on threshold regression#3515
rnro merged 1 commit intoapple:mainfrom
rnro:benchmarks_script_missing_diffs

Conversation

@rnro
Copy link
Copy Markdown
Contributor

@rnro rnro commented Feb 20, 2026

Motivation

  • check_benchmark_thresholds.sh was updated to check for exit code 2 from swift package benchmark thresholds check to distinguish threshold regression from build errors.
  • It seems SwiftPM's CommandPlugin infrastructure always exits with code 1 when performCommand throws, regardless of the error's raw value. So the exit code is never 2, causing all regressions to fall through to the else branch and be misreported as build errors, with no diff output.

Modifications

  • Remove the rc == 2 check and instead attempt thresholds update for any non-zero rc from thresholds check.
  • Use the result of thresholds update to distinguish regression (success) from build error (failure).
  • Remove --exit-code from git diff to prevent set -uo pipefail from aborting the script before output is fully flushed.

Result

  • Benchmark threshold regressions correctly output the === BEGIN DIFF === section again.
  • Actual build errors are still correctly detected and reported.

Motivation

* `check_benchmark_thresholds.sh` was updated to check for exit code 2
from `swift package benchmark thresholds check` to distinguish
threshold regression from build errors.
* It seems SwiftPM's `CommandPlugin` infrastructure always exits with code 1 when
`performCommand` throws, regardless of the error's raw value. So the
exit code is never 2, causing all regressions to fall through to the
`else` branch and be misreported as build errors, with no diff output.

Modifications

* Remove the `rc == 2` check and instead attempt `thresholds update`
for any non-zero `rc` from `thresholds check`.
* Use the result of `thresholds update` to distinguish regression
(success) from build error (failure).
* Remove `--exit-code` from `git diff` to prevent `set -uo pipefail`
from aborting the script before output is fully flushed.

Result

* Benchmark threshold regressions correctly output the `=== BEGIN DIFF
===` section again.
* Actual build errors are still correctly detected and reported.
@rnro rnro added the semver/none No version bump required. label Feb 20, 2026
@rnro rnro enabled auto-merge (squash) February 20, 2026 15:36
Copy link
Copy Markdown
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely more resilient than my solution.

@rnro rnro disabled auto-merge February 20, 2026 15:45
@rnro rnro enabled auto-merge (squash) February 20, 2026 15:50
@rnro rnro merged commit 83b0fcd into apple:main Feb 20, 2026
55 of 56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants