-
Notifications
You must be signed in to change notification settings - Fork 22
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
ci: fix the 32 byte inaccuracy of SDK size reports #709
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
0c5b750
to
9eb8df9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #709 +/- ##
=======================================
Coverage 56.93% 56.93%
=======================================
Files 139 139
Lines 3869 3869
=======================================
Hits 2203 2203
Misses 1666 1666 ☔ View full report in Codecov by Sentry. |
8a7d447
to
b16c575
Compare
|
|
fd70fa1
to
7ee4a11
Compare
SDK binary size reports 📊SDK binary size of this PR
SDK binary size diff report between this PR and the main branch
|
7ee4a11
to
dd39586
Compare
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.
All of the logic in the steps
came from workflows/build-sample-apps.yml
. Moved logic into this action so it can be re-used in generate-sdk-size-report
.
|
||
inputs: |
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.
This action used to take in an already built sample app. Now it builds it's own. So the inputs needed to change.
GH actions cannot reference secrets. So the only input in this action is a required secret that fastlane uses for code signing the compiled sample app.
- name: Make a build of the sample app to generate report for | ||
uses: ./.github/actions/build-sample-app | ||
id: build-sample-app | ||
with: | ||
apn-or-fcm: 'APN' | ||
sample-app: 'APN-UIKit' | ||
# Pass in a hard-coded version and build number to ensure that all sample app builds that are compiled for SDK size reports are consistent. | ||
# When we compare size reports between different builds, we want to ensure that the only difference is the SDK code that was modified in a PR. | ||
fastlane-build-args: '{"app_version": "1.0.0", "build_number": "1"}' | ||
# workspace credentials do not matter since we are not using this app build. | ||
customerio-workspace-siteid: "12345" | ||
customerio-workspace-cdp-api-key: "12345" | ||
GOOGLE_CLOUD_MATCH_READONLY_SERVICE_ACCOUNT_B64: ${{ inputs.GOOGLE_CLOUD_MATCH_READONLY_SERVICE_ACCOUNT_B64 }} |
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.
Just like in workflows/build-sample-app
, build the APN sample app here. But, pass in a hard-coded app version and build number which is what fixes the 32 byte inaccuracy problem.
@@ -164,8 +98,21 @@ jobs: | |||
* ${{ matrix.sample-app }}: Build failed. See [CI job logs](https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}) to determine the issue and try re-building. | |||
edit-mode: append # append new line to the existing PR comment to build a list of all sample app builds. | |||
|
|||
generate-sdk-size-reports: |
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.
Moved SDK size report generation out of the build-sample-apps
job and into it's own job. This is for CI performance so SDK reports can be made in parallel to building sample apps.
|
||
- name: Install CLI tools used in CI script |
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.
A lot of this code got moved into the re-usable build-sample-app
action. Making this workflow shorter 🎊
Part of: https://linear.app/customerio/issue/MBL-155/track-the-binary-size-of-our-ios-sdk-upon-every-release As called out in the ticket, the SDK size report comparing main branch to open PRs is inaccurate by 32 bytes. This commit fixes this so if you open a PR and not modify any code in it, the diff should show 0 bytes difference. Testing consideration: * The PR should see SDK size reports generated as before. QA sample app builds created and uploaded as before. * We cannot fully test this until we merge into main and the main branch creates a new cache. Notes for reviewer: * I fixed this issue by setting up the CI to compile a version of the APN sample app that does not modify the Version.swift file. This compiled app is used to generate SDK size report. The CI continues to compile sample apps for QA testing and it should work as previously expected. * Because the generate-sdk-size-report action is now compiling sample apps, I created a new action, build-sample-app, to make building a sample app re-usable. * "fastlane ios build" command now takes optional arguments to configure it. QA sample apps will make builds differently then builds for generating SDK size reports. commit-id:eddb8612
dd39586
to
9895079
Compare
Part of: https://linear.app/customerio/issue/MBL-155/track-the-binary-size-of-our-ios-sdk-upon-every-release
As called out in the ticket, the SDK size report comparing main branch to open PRs is inaccurate by 32 bytes. This commit fixes this so if you open a PR and not modify any code in it, the diff should show 0 bytes difference.
Testing consideration:
Notes for reviewer:
commit-id:eddb8612