test(android): Guard reactNativeArchitectures fix with a subset-ABI build#6416
Open
antonis wants to merge 1 commit into
Open
test(android): Guard reactNativeArchitectures fix with a subset-ABI build#6416antonis wants to merge 1 commit into
antonis wants to merge 1 commit into
Conversation
…uild Closes the last CI coverage gap around libsentry-tm-perf-logger.so. The 8.17.2 fix made `packages/core/android/build.gradle` honour the host app's `reactNativeArchitectures` (root cause of #6398), but nothing in CI exercises a subset-ABI build. The existing all-ABI builds pass identically with or without the fix — they can't catch a regression to hardcoded abiFilters. Split the `build-android` matrix so each ABI-coverage concern gets its own entry: - `dev` → subset (arm64-v8a only) + a new guard asserting the module packaged libsentry-tm-perf-logger.so for exactly that ABI. Regresses the abiFilters fix → build produces extra ABIs → guard fails. - `production` → all four ABIs (unchanged), keeping the 16 KB alignment check running across every ABI (#6394 guard). Downstream jobs are unaffected: Archive/Upload Android APK and test-android (Maestro E2E) are gated on `matrix.build-type == 'production'`, which still builds all ABIs. Verified locally against a real subset APK from the sample: - packaged: only lib/arm64-v8a/libsentry-tm-perf-logger.so → guard passes - simulated regression (unrequested armeabi-v7a .so leaks in) → guard fails with clear message - missing lib case → distinct "not found" error Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 752153f. Configure here.
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1122a96+dirty | 422.22 ms | 464.33 ms | 42.10 ms |
| 1a5721e+dirty | 424.07 ms | 482.32 ms | 58.25 ms |
| 5c1e987+dirty | 423.52 ms | 471.64 ms | 48.12 ms |
| b04af96+dirty | 496.48 ms | 557.88 ms | 61.40 ms |
| e763471+dirty | 426.57 ms | 458.21 ms | 31.64 ms |
| 20fbd51+dirty | 487.89 ms | 535.60 ms | 47.71 ms |
| a5d243c+dirty | 424.52 ms | 485.18 ms | 60.66 ms |
| 5257d80+dirty | 423.37 ms | 467.54 ms | 44.17 ms |
| ab203f9+dirty | 447.04 ms | 507.43 ms | 60.38 ms |
| f9c1ed4+dirty | 431.00 ms | 466.22 ms | 35.22 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1122a96+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 1a5721e+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 5c1e987+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| b04af96+dirty | 49.74 MiB | 55.00 MiB | 5.26 MiB |
| e763471+dirty | 49.74 MiB | 54.85 MiB | 5.11 MiB |
| 20fbd51+dirty | 49.74 MiB | 54.81 MiB | 5.07 MiB |
| a5d243c+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 5257d80+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| ab203f9+dirty | 49.74 MiB | 54.85 MiB | 5.11 MiB |
| f9c1ed4+dirty | 49.74 MiB | 54.86 MiB | 5.12 MiB |
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 580fb5c+dirty | 3836.13 ms | 1218.72 ms | -2617.41 ms |
| d0e3b3e+dirty | 3854.77 ms | 1218.58 ms | -2636.19 ms |
| 37a2091+dirty | 3821.77 ms | 1212.34 ms | -2609.43 ms |
| 5789645+dirty | 3841.18 ms | 1220.67 ms | -2620.52 ms |
| 4e0ba9c+dirty | 3839.22 ms | 1221.06 ms | -2618.16 ms |
| 822d35b+dirty | 3830.64 ms | 1209.20 ms | -2621.44 ms |
| 0b5a379+dirty | 3828.91 ms | 1214.12 ms | -2614.79 ms |
| 68672fc+dirty | 3841.58 ms | 1228.89 ms | -2612.69 ms |
| 2c735cc+dirty | 1229.67 ms | 1221.50 ms | -8.17 ms |
| 7ff4d0f+dirty | 3838.29 ms | 1225.13 ms | -2613.16 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 580fb5c+dirty | 4.98 MiB | 6.46 MiB | 1.48 MiB |
| d0e3b3e+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 37a2091+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| 5789645+dirty | 4.98 MiB | 6.50 MiB | 1.52 MiB |
| 4e0ba9c+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 822d35b+dirty | 4.98 MiB | 6.50 MiB | 1.53 MiB |
| 0b5a379+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| 68672fc+dirty | 5.15 MiB | 6.71 MiB | 1.55 MiB |
| 2c735cc+dirty | 3.38 MiB | 4.74 MiB | 1.35 MiB |
| 7ff4d0f+dirty | 5.15 MiB | 6.70 MiB | 1.55 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0b1b5e3+dirty | 3820.72 ms | 1207.94 ms | -2612.78 ms |
| d0e3b3e+dirty | 3846.64 ms | 1226.27 ms | -2620.38 ms |
| 0d9949d+dirty | 1203.94 ms | 1202.27 ms | -1.67 ms |
| 5789645+dirty | 3841.36 ms | 1214.81 ms | -2626.55 ms |
| acd838e+dirty | 3835.94 ms | 1215.87 ms | -2620.07 ms |
| 822d35b+dirty | 3841.52 ms | 1221.75 ms | -2619.77 ms |
| 267d3ed+dirty | 3860.14 ms | 1223.39 ms | -2636.76 ms |
| f9c1ed4+dirty | 3842.09 ms | 1220.70 ms | -2621.40 ms |
| 8929511+dirty | 1223.41 ms | 1222.49 ms | -0.92 ms |
| 9b5615c+dirty | 3837.21 ms | 1215.33 ms | -2621.89 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0b1b5e3+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| d0e3b3e+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 0d9949d+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 5789645+dirty | 4.98 MiB | 6.50 MiB | 1.52 MiB |
| acd838e+dirty | 5.15 MiB | 6.70 MiB | 1.55 MiB |
| 822d35b+dirty | 4.98 MiB | 6.50 MiB | 1.53 MiB |
| 267d3ed+dirty | 5.15 MiB | 6.69 MiB | 1.54 MiB |
| f9c1ed4+dirty | 4.98 MiB | 6.50 MiB | 1.53 MiB |
| 8929511+dirty | 3.38 MiB | 4.80 MiB | 1.42 MiB |
| 9b5615c+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Closes the last CI coverage gap around the app-compiled
libsentry-tm-perf-logger.sonative library. Follow-up to the 8.17.2 fixes (#6406, #6407).The 8.17.2 root-cause fix for #6398 made
packages/core/android/build.gradlehonour the host app'sreactNativeArchitectures(abiFilters(*reactNativeArchitectures()), matching react-native-screens/reanimated/the RN template). But nothing in CI exercises a subset-ABI build, so the existing all-ABI builds pass identically with or without the fix — they cannot catch a regression back to hardcodedabiFilters.This PR splits the bare-RN sample
build-androidmatrix so each ABI-coverage concern gets its own entry, and adds a lightweight assertion:devarm64-v8aonly (subset)check-tm-perf-logger-abi-subset.shasserts the packaged.soset matches exactlyreactNativeArchitecturesfix (#6398 root cause)productionThe new script
scripts/check-tm-perf-logger-abi-subset.shunzips a built APK/AAB, findslibsentry-tm-perf-logger.soper ABI, and asserts the set matches the expected CSV exactly. Same shape as the existingcheck-android-16kb-alignment.sh.💡 Motivation and Context
Both #6394 and #6398 escaped because CI only built x86; this module is the SDK's only library compiled per-ABI on the user's side. #6406 + #6407 fixed the code paths; this closes the CI side by ensuring a future regression of the
reactNativeArchitecturesfix is caught pre-merge.Downstream jobs are unaffected:
Archive Android App/Upload Android APK/test-android(Maestro E2E) are all gated onmatrix.build-type == 'production', which still builds all four ABIs. Build time for thedeventry actually drops slightly (1 ABI vs 4).💚 How did you test it?
Verified locally against a real subset APK from the RN sample (
./scripts/build-android.sh -PreactNativeArchitectures=arm64-v8a,SENTRY_DISABLE_AUTO_UPLOAD=true):arm64-v8aarm64-v8aarmeabi-v7a.soleaks inarm64-v8aarm64-v8a armeabi-v7a.somissing entirelyarm64-v8aWorkflow YAML validated with
ruby -e 'YAML.load_file(...)'; shell script passesbash -n.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
🤖 Generated with Claude Code