Repository navigation
[SDK-790] ci: run CI on master and run Android bridge unit tests - #919
Conversation
… compiles against native 3.10.1 The published iterableapi 3.10.1 AAR is compiled with Kotlin 1.9 without jvm-default, so onEmbeddedMessagingSyncSucceeded and onEmbeddedMessagingSyncFailed are abstract to Java implementers. RNIterableAPIModuleImpl did not override them, so the module failed javac. Add no-op overrides that match the native defaults.
Iterable.inAppManager.showMessage called IterableInAppManager.showMessage on the React Native native-modules thread. Since native Android 3.9.0, a host activity that is not a FragmentActivity takes the Dialog path, whose LifecycleRegistry.addObserver call throws IllegalStateException off the main thread and kills the process. - Post the native call with UiThreadUtil.runOnUiThread, matching setAutoDisplayPaused and the native iOS SDK. - Reject when the message id is missing instead of passing null into native (NPE). - Resolve null when the message is dismissed without a URL instead of NPE on toString. - Fix the empty messageId check, which used reference equality. Adds the first JVM unit tests for the Android bridge (Robolectric + Mockito, versions mirrored from iterable-android-sdk) and excludes them from the npm package.
ci.yml triggered on main while the default branch is master, so it has never run. Add a job that runs the Android bridge JVM tests on every PR.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
|
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
…yarn React Native 0.85 requires Xcode 16.1+, macos-14 ships 15.4. Mirror the CodeQL Swift job. Run the Android bridge tests via an example script so autolinking finds the RN CLI.
| name: Android bridge unit tests | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Setup | ||
| uses: ./.github/actions/setup | ||
|
|
||
| - name: Install JDK | ||
| uses: actions/setup-java@v3 | ||
| with: | ||
| distribution: 'zulu' | ||
| java-version: '17' | ||
|
|
||
| - name: Finalize Android SDK | ||
| run: | | ||
| /bin/bash -c "yes | $ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager --licenses > /dev/null" | ||
|
|
||
| - name: Cache Gradle | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: | | ||
| ~/.gradle/wrapper | ||
| ~/.gradle/caches | ||
| key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/gradle/wrapper/gradle-wrapper.properties') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gradle- | ||
|
|
||
| - name: Run Android bridge unit tests | ||
| run: yarn example test:android |
…cache The tasks run in the example workspace, so inputs resolve relative to example/ and directory names without ** match no files. The cache only hashed example/package.json and example/src, so SDK android/ and ios/ changes would hit the cache and skip native builds. Also hash android/build.gradle into the Gradle cache key for the bridge test job.
|
Review follow-up: fixed |
The CocoaPods cache restores Pods but not the codegen output in ios/build/generated, so skipping pod install on a cache hit left ReactAppDependencyProvider missing.
|
Second latent CI bug, surfaced on the run after the turbo fix: |
jferrao-itrbl
left a comment
jferrao-itrbl
left a comment
There was a problem hiding this comment.
Branch protection on master: a red build still does not block merge
What the code does: .github/workflows/ci.yml runs on master, and the new Android test job is in .github/workflows/unit-test.yml. Branch protection for master still has required_status_checks.contexts: [] and checks: [].
What the spec says: SDK-790 says that when CI runs on a change that breaks Android compilation, the pull request is blocked, and the required checks on master include the Android and iOS builds.
Why it conflicts: The jobs run and can go red. Merge is still allowed. The pull request describes this as an admin follow-up, so the criterion is unchanged since the last review.
Suggested action: An admin marks build-android, build-ios, and Android bridge unit tests required on master before SDK-790 is closed. lint already covers typecheck.
| "!../android/build/**", | ||
| "!../android/.gradle/**", | ||
| "!../android/.cxx/**", | ||
| "../src/*.ts", | ||
| "../src/*.tsx", |
There was a problem hiding this comment.
Nested library sources and the example app are still outside the cache hash
Also in ~L31–32
What the code does: build:android and build:ios hash ../src/*.ts and ../src/*.tsx only, one directory deep. A dry run includes ../src/index.tsx and excludes ../src/api/NativeRNIterableAPI.ts and ../src/core/classes/Iterable.ts. The same edit removed the example-package src/*.ts and src/*.tsx inputs, so example/src/index.tsx and example/src/NativeJwtTokenModule.ts are no longer hashed either.
What the spec says: A change that breaks Android compilation must fail CI. The example build is the compile gate for that.
Why it conflicts: Native android/** and ios/** now bust the cache, so the 3.2.0 javac failure would miss and rebuild. A change under src/ that breaks codegen, or a change under example/src that breaks the example bundle, still hashes the same and build-android / build-ios can replay a hit.
Suggested action: Hash the library source tree that codegen compiles, and hash the example app sources the example build compiles. Confirm with a dry run that src/api/NativeRNIterableAPI.ts and example/src/index.tsx show up as inputs.
There was a problem hiding this comment.
Fixed in 014eced. Both tasks now hash ../src/**/*.ts(x) and the example's src/**/*.ts(x), index.js, app.json and react-native.config.js (autolinking). Dry run confirms ../src/api/NativeRNIterableAPI.ts (codegen jsSrcsDir), ../src/core/classes/Iterable.ts, src/index.tsx and src/NativeJwtTokenModule.ts are inputs for both build:android (232 files) and build:ios (223 files), with no build/Pods/.gradle/.cxx outputs.
Inputs only covered src/*.ts one level deep and dropped example/src, so codegen spec or example app changes could replay a cached native build. Hash src/** for the library and the example, plus the example entry points and autolinking config.
|
@jferrao-itrbl on branch protection: agreed it has to happen before SDK-790 closes, and I have admin to do it. I'm sequencing it rather than doing it now: until this PR merges, |
3 participants
📝 Summary
Make
ci.ymlrun onmaster(it has never run) and run the Android bridge unit tests on every PR.🎟️ Jira Ticket: SDK-790
📖 Description
ci.ymltriggered onmain, but this repo's default branch ismaster. The workflow has 0 runs since the Sept 2024 scaffold (a5677631), so lint, typecheck, circular check, build-library, build-android and build-ios have never gated a PR. That is how 3.2.0 shipped an Android module that does not compile (fixed in #918).ci.yml:maintomasterforpushandpull_request.unit-test.yml: newAndroid bridge unit testsjob running:iterable_react-native-sdk:testDebugUnitTest(JDK 17, same setup steps asbuild-android).CONTRIBUTING.md: how to run JS and Android bridge tests locally.Stacked on #918. This branch is cut from #918 so
build-androidand the new test job can pass. Until #918 merges the diff here includes its two commits; it shrinks to the three files above once #918 lands.This is the first time
ci.ymlwill ever run, so any jobs that fail on this PR are existing breakage that was never visible. I'll fix them in follow-up commits here or call them out.Repo settings follow-up (needs an admin):
masterhas no required status checks (required_status_checks.contextsis[]). After this is green, markbuild-android,build-ios,lintandAndroid bridge unit testsas required.🧪 How to test?
CI on this PR is the test: the
CIworkflow should appear in the checks for the first time.🧾 Changelog
No customer facing changes