-
-
Notifications
You must be signed in to change notification settings - Fork 372
chore(ci): Add required checks for benchmarking #5996
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
chore(ci): Add required checks for benchmarking #5996
Conversation
Co-authored-by: phil.niedertscheider <phil.niedertscheider@sentry.io>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5996 +/- ##
=============================================
+ Coverage 86.589% 86.735% +0.146%
=============================================
Files 424 424
Lines 36710 36723 +13
Branches 15739 17362 +1623
=============================================
+ Hits 31787 31852 +65
+ Misses 4881 4824 -57
- Partials 42 47 +5 see 20 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
itaybre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ired-checks-for-benchmarking-923d
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a2a3bfb | 1227.94 ms | 1261.26 ms | 33.32 ms |
| b9aacb6 | 1230.42 ms | 1251.00 ms | 20.58 ms |
| 51b7dd3 | 1235.06 ms | 1258.21 ms | 23.15 ms |
| 7d23639 | 1237.93 ms | 1243.04 ms | 5.11 ms |
| c03a8d8 | 1234.77 ms | 1259.19 ms | 24.42 ms |
| 42a95d5 | 1206.00 ms | 1224.26 ms | 18.26 ms |
| b57ee62 | 1218.21 ms | 1248.94 ms | 30.73 ms |
| ebc3a34 | 1236.24 ms | 1261.02 ms | 24.78 ms |
| 5196f0d | 1213.35 ms | 1231.37 ms | 18.02 ms |
| ae7be93 | 1236.24 ms | 1258.18 ms | 21.94 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a2a3bfb | 23.75 KiB | 872.67 KiB | 848.92 KiB |
| b9aacb6 | 23.75 KiB | 913.64 KiB | 889.89 KiB |
| 51b7dd3 | 23.75 KiB | 913.26 KiB | 889.52 KiB |
| 7d23639 | 23.75 KiB | 891.01 KiB | 867.26 KiB |
| c03a8d8 | 23.75 KiB | 928.15 KiB | 904.40 KiB |
| 42a95d5 | 23.75 KiB | 906.08 KiB | 882.33 KiB |
| b57ee62 | 23.75 KiB | 912.47 KiB | 888.72 KiB |
| ebc3a34 | 23.75 KiB | 919.91 KiB | 896.16 KiB |
| 5196f0d | 23.75 KiB | 876.93 KiB | 853.19 KiB |
| ae7be93 | 23.75 KiB | 879.24 KiB | 855.49 KiB |
📜 Description
Migrates the
benchmarking.ymlworkflow to usedorny/paths-filterfor conditional execution on pull requests.run_benchmarking_for_prsis added to.github/file-filters.yml.on.pull_request.pathsfilter is removed frombenchmarking.yml.files-changedjob is introduced to detect relevant file changes.build-benchmark-test-targetandrun-ui-tests-with-saucejobs are now conditional based on thefiles-changedoutput.benchmarking-required-checkjob is added, which always runs to serve as a reliable required check for branch protection.💡 Motivation and Context
This change is part of a larger effort (see #5951) to migrate all workflows using
on.[event].pathsfiltering to afiles-changedjob withdorny/paths-filter. This allows for more robust required checks in branch protection rules.For the benchmarking workflow, this ensures:
benchmarking-required-checkis always available for branch protection, regardless of whether the main benchmark jobs are skipped.This follows the pattern established in #5893 for other workflows.
Fixes #5955
💚 How did you test it?
The changes were verified by observing the CI behavior on this pull request, ensuring the
files-changedjob correctly identifies relevant changes and the main jobs run or skip as expected, and thebenchmarking-required-checkjob always completes.📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.#skip-changelog
[skip ci]