Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error enum values not propagated as exit codes for benchmark comparison in CI. #235

Open
loonatick-src opened this issue Mar 18, 2024 · 5 comments

Comments

@loonatick-src
Copy link

From the docs:

These baselines can then be checked with:

swift package benchmark baseline check --check-absolute-path /relative/or/absolute/path/to/Thresholds

The absolute check will have an exit code of 0 if the check is exactly equal, but will return with exit code 2 if there were any regressions or exit code 4 if there were only improvements.

But, this does not seem to be the case; this is what I saw.

$ swift package benchmark baseline check main merge_request              
Building for debugging...
[1/1] Write swift-version-6E0725D62189FA0A.txt
Build complete! (0.29s)
Building for debugging...
[1/1] Write swift-version-6E0725D62189FA0A.txt
Build complete! (0.66s)
Build complete!
Building BenchmarkTool in release mode...

=======================================================================================
Threshold deviations for test benchmarks:TestBenchmarks
=======================================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Time (wall clock) (μs, %)                │            main │   merge_request │    Difference % │     Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p75                                      │             185 │             170 │              -8 │               5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛

New baseline 'merge_request' is BETTER than the 'main' baseline thresholds.

error: benchmarkThresholdImprovement
$ echo $?
1

The return code for improvement is 1, and from the docs I expected it to be 4 in the CI.

I see that the error enum's value is defined as 4 here, and it is thrown from performCommand.

I suspect that performCommand's caller might not be calling exit with the error enum's raw value as done here.

 Diagnostics.error(String(describing: error))
 exit(1)

This seems to be happening in the example benchmark comparison run. I couldn't completely trace the call across library boundaries. Is this a bug or am I missing something?

Environment

  • OS: macOS Sonoma 14.4
  • package-benchmark version: 1.22.3
  • Swift version: 5.11-dev, DEVELOPMENT-SNAPSHOT-2023-12-07-a
@loonatick-src
Copy link
Author

Ah well I just saw that this has already an open issue upstream , if I correctly understand that this is the same problem.

@hassila
Copy link
Contributor

hassila commented Mar 18, 2024

Yeah, it's the same issue. It's possible to work around by running the benchmark tool directly without swiftpm - running the benchmark with --debug you can see the actual command line required to drive the benchmark tool and instead run that in ci, the you should get the correct error code surfaced.

@loonatick-src
Copy link
Author

loonatick-src commented Mar 19, 2024

Is this also reflected in the docs? I could not find anything of the sort and was going entirely by the 0/2/4 exit codes for swift package benchmark baseline check.

@hassila
Copy link
Contributor

hassila commented Mar 27, 2024

Thanks for the report, unfortunately away for a couple of weeks and won't have time to check on this until then, sorry for delay.

@loonatick-src
Copy link
Author

No worries! Also thank you for pointing out the workaround

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants