Repository navigation
[No QA] test(ios): Verify NitroSQLite database directory in native CI - #102980
chrispader wants to merge 3 commits into
Conversation
|
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@MelvinBot could you please create an issue for this PR (the original propsal: https://github.com/Expensify/Mobile-Expensify/pull/14129#discussion_r4160094962) and assign me and @Julesssss to it. Link to the issue in this PR |
|
I created Verify NitroSQLite iOS database directory in native CI and assigned you and In this PR's Fixed Issues section, I replaced the closed patch-tracking issue ( I couldn't open the proposal comment, so I wrote the issue from this PR's description. Please edit the issue if the proposal says something different. |
Reviewer Checklist
Screenshots/VideosNo manual app testing or screenshots/videos are needed because this PR only adds CI verification. We ran the native script with valid and invalid configurations, including a simulated dependency change; all behaved as expected. The CI verification also passed. |
|
friendly bump @Julesssss since you requested this 🙌🏼 |
cc @Julesssss
NitroSQLite falls back to Documents when its native code no longer recognizes the database-location plist key. The existing plist assertions would still pass after such a dependency upgrade, even though HybridApp exposes Documents through Files. I added a native regression check to the existing macOS verification job, now named Verify iOS native configuration, so dependency upgrades must still honor the Application Support setting.
Explanation of Change
The check compiles the installed package's unchanged
ios/OnLoad.mmand runs its iOS directory-selection branch with the host plist in a temporary bundle. Minimal bridge declarations expose the selected native directory without requiring a React Native build. The test asserts that the directory remains Application Support and reports the actual selection on failure.CI uses NewDot's plist, which configures the same key and value already guarded in HybridApp's plist check. This avoids needing private submodule credentials on fork pull requests. The runner also accepts other host plists for local verification. The existing job continues to run on dependency and iOS changes, and now also runs when the native test, this workflow, or NitroSQLite patches change.
Local native checks passed against the published NitroSQLite 9.8.3 package, whose archive integrity matches the lockfile, using both NewDot and HybridApp plists. A dependency copy with the key renamed failed while the host plist remained unchanged. Missing keys, Documents values, and invalid values also failed. This checks the native directory selector; full app startup and database migration remain separate coverage.
Fixed Issues
$ #103804
PROPOSAL: https://github.com/Expensify/Mobile-Expensify/pull/14129#discussion_r4160094962
Reproduction
RNNitroSQLite_DatabaseLocationvalue set toApplicationSupport.ios/OnLoad.mmso it no longer matches the host plist.Tests
Manual app tests are not applicable because this only adds CI verification. The native regression checks and their results are described above.
Offline tests
Offline app testing is not applicable because this does not change runtime app behavior.
QA Steps
No staging or production QA is required for this CI-only change.
PR Author Checklist
I have considered every checklist item. Manual app, offline, high-traffic, console, screenshot, and UI/component checks do not apply to this CI-only change. I marked those items complete as not applicable; the native regression results are described above.
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
No screenshots are attached. This change only adds native CI verification and has no UI changes.