-
Notifications
You must be signed in to change notification settings - Fork 377
ci: [SDK-4690] add Android E2E workflow #2652
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
Open
fadi-george
wants to merge
3
commits into
main
Choose a base branch
from
fadi/sdk-4690
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| name: 'Setup Demo' | ||
| description: 'Installs the JDK and Android toolchains, caches Gradle, and writes the demo local.properties file' | ||
| inputs: | ||
| onesignal-app-id: | ||
| description: 'OneSignal App ID written into examples/demo/local.properties' | ||
| required: true | ||
| onesignal-api-key: | ||
| description: 'OneSignal REST API key written into examples/demo/local.properties' | ||
| required: true | ||
| runs: | ||
| using: 'composite' | ||
| steps: | ||
| - name: Set up Java | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: temurin | ||
| java-version: '17' | ||
|
|
||
| - name: Set up Android SDK | ||
| uses: android-actions/setup-android@v3 | ||
| with: | ||
| cmdline-tools-version: '10406996' | ||
| log-accepted-android-sdk-licenses: false | ||
|
|
||
| - name: Cache Gradle | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.gradle/caches | ||
| ~/.gradle/wrapper | ||
| key: ${{ runner.os }}-gradle-demo-${{ hashFiles('examples/demo/**/*.gradle*', 'examples/demo/**/gradle-wrapper.properties') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gradle-demo- | ||
| ${{ runner.os }}-gradle- | ||
|
|
||
| # Mirrors the Capacitor demo's `.env`: writes the override file before any | ||
| # build runs so the OneSignal App ID + channel id end up baked into | ||
| # BuildConfig. `examples/demo/app/build.gradle.kts` resolves each key in | ||
| # this order: `-PKEY=value` from the CLI > local.properties > built-in | ||
| # default, so writing to local.properties is the closest equivalent to | ||
| # the Capacitor `.env` flow. | ||
| - name: Write demo local.properties | ||
| shell: bash | ||
| working-directory: examples/demo | ||
| env: | ||
| ONESIGNAL_APP_ID: ${{ inputs.onesignal-app-id }} | ||
| ONESIGNAL_API_KEY: ${{ inputs.onesignal-api-key }} | ||
| run: | | ||
| { | ||
| echo "ONESIGNAL_APP_ID=${ONESIGNAL_APP_ID}" | ||
| echo "ONESIGNAL_API_KEY=${ONESIGNAL_API_KEY}" | ||
| echo "ONESIGNAL_ANDROID_CHANNEL_ID=7ec2ece9-c538-4656-9516-1316f48a005c" | ||
| } > local.properties | ||
|
Check failure on line 53 in .github/actions/setup-demo/action.yml
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| name: E2E Tests | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - rel/** | ||
| workflow_dispatch: | ||
| inputs: | ||
| sdk-version: | ||
| description: 'OneSignal Android SDK version to wait for and build against (defaults to OneSignalSDK/gradle.properties).' | ||
| required: false | ||
| type: string | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| build-android: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up demo | ||
| uses: ./.github/actions/setup-demo | ||
| with: | ||
| onesignal-app-id: ${{ vars.APPIUM_ONESIGNAL_APP_ID }} | ||
| onesignal-api-key: ${{ secrets.APPIUM_ONESIGNAL_API_KEY }} | ||
|
|
||
| - name: Resolve OneSignal Android SDK version | ||
| id: android-sdk-version | ||
| env: | ||
| INPUT_VERSION: ${{ github.event.inputs.sdk-version }} | ||
| run: | | ||
| if [ -n "$INPUT_VERSION" ]; then | ||
| VERSION="$INPUT_VERSION" | ||
| else | ||
| VERSION=$(grep -E '^SDK_VERSION=' OneSignalSDK/gradle.properties | cut -d '=' -f2) | ||
| fi | ||
| if [ -z "$VERSION" ]; then | ||
| echo "::error::Could not resolve OneSignal Android SDK version" | ||
| exit 1 | ||
| fi | ||
| echo "Resolved OneSignal Android SDK version: $VERSION" | ||
| echo "version=${VERSION}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Wait for OneSignal Android SDK on Maven Central | ||
| uses: OneSignal/sdk-shared/.github/actions/wait-for-maven-artifact@main | ||
| with: | ||
| version: ${{ steps.android-sdk-version.outputs.version }} | ||
|
|
||
| - name: Build debug APK | ||
| working-directory: examples/demo | ||
| # Build the gms-debug variant: BrowserStack devices all run Google | ||
| # Play Services, and debug ensures isDebuggable=true so Appium's | ||
| # UiAutomator2 driver can attach. The Maven-resolved OneSignal | ||
| # dependency is pinned to the version we just waited for, so the | ||
| # APK exercises the actual published artifact. | ||
| run: ./gradlew :app:assembleGmsDebug -PSDK_VERSION=${{ steps.android-sdk-version.outputs.version }} --console=plain --warning-mode=summary | ||
|
|
||
| - name: Upload APK | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: demo-apk | ||
| path: examples/demo/app/build/outputs/apk/gms/debug/app-gms-debug.apk | ||
| retention-days: 1 | ||
| compression-level: 0 | ||
|
|
||
| e2e-android: | ||
| needs: build-android | ||
| uses: OneSignal/sdk-shared/.github/workflows/appium-e2e.yml@main | ||
| secrets: inherit | ||
| with: | ||
| platform: android | ||
| app-artifact: demo-apk | ||
| app-filename: app-gms-debug.apk | ||
| sdk-type: android | ||
| build-name: android-${{ github.ref_name }}-${{ github.run_number }} |
Oops, something went wrong.
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.
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.
🔴 The composite action accepts a required
onesignal-api-keyinput and writesONESIGNAL_API_KEY=...toexamples/demo/local.properties, but nothing ever reads it:examples/demo/app/build.gradle.ktsonly callsdemoOverride()forONESIGNAL_APP_IDandONESIGNAL_ANDROID_CHANNEL_ID, and a repo-wide grep confirmsONESIGNAL_API_KEYappears only in the two new CI files. The required input therefore forces every caller to supply an unused REST API key and silently materializes it onto the runner disk for no benefit. Either drop theonesignal-api-keyinput and the correspondingecholine ataction.yml:51(plus thesecrets.APPIUM_ONESIGNAL_API_KEYwiring ate2e.yml:32), or wire the value throughbuild.gradle.ktsto a real consumer (e.g.,BuildConfig).Extended reasoning...
What the bug is
.github/actions/setup-demo/action.ymldeclaresonesignal-api-keyas a required input (lines 7-9) and writes it intoexamples/demo/local.properties(line 51). The accompanying comment at lines 36-41 describes the file as containing only "the OneSignal App ID + channel id" — there is no mention of an API key, suggesting the API-key line was added by accident or as leftover boilerplate from the Capacitor template.Why it's dead plumbing
examples/demo/app/build.gradle.ktsis the only consumer oflocal.propertiesin this repo. ItsdemoOverride()helper is invoked exactly twice — forONESIGNAL_APP_ID(line 57) andONESIGNAL_ANDROID_CHANNEL_ID(line 64). There is nodemoOverride("ONESIGNAL_API_KEY")call, and a repo-wide grep forONESIGNAL_API_KEYreturns only the two new CI files (.github/actions/setup-demo/action.ymland.github/workflows/e2e.yml). No Gradle script, Kotlin/Java source file, or AndroidManifest references the key.Why the Appium suite can't pick it up either
local.propertiesis a Gradle-configuration-time file that lives on the CI runner's filesystem; it is not packaged into the APK. The APK uploaded to BrowserStack therefore never sees the key, and the sharedOneSignal/sdk-shared/.github/workflows/appium-e2e.yml@mainworkflow obtains its OneSignal/BrowserStack secrets directly viasecrets: inheritone2e.yml:48— not vialocal.properties. So there is no plausible runtime path by which the written value reaches any consumer.Step-by-step proof
.github/workflows/e2e.yml:32passessecrets.APPIUM_ONESIGNAL_API_KEYto the composite action'sonesignal-api-keyinput..github/actions/setup-demo/action.yml:51writesONESIGNAL_API_KEY=$ONESIGNAL_API_KEYtoexamples/demo/local.properties../gradlew :app:assembleGmsDebugruns (e2e.yml:69). Gradle parsesbuild.gradle.kts, which only callsdemoOverride("ONESIGNAL_APP_ID")anddemoOverride("ONESIGNAL_ANDROID_CHANNEL_ID"). TheONESIGNAL_API_KEYline inlocal.propertiesis ignored.app-gms-debug.apkis uploaded asdemo-apkand consumed by the reusable Appium workflow. The APK contains noONESIGNAL_API_KEYvalue, andlocal.propertiesis not shipped with it.secrets: inherit, independently of anything in this repo's build.The key is therefore written to a file nothing reads, on a runner that is torn down at the end of the job.
Impact
Severity is normal, not a runtime crash but a real correctness/cleanliness issue:
BuildConfig.ONESIGNAL_API_KEY(or similar) is available — it isn't.required: true, any caller withoutAPPIUM_ONESIGNAL_API_KEYset will fail the workflow at thewith:validation step for zero benefit.How to fix
Either:
onesignal-api-keyinput (action.yml:7-9), theONESIGNAL_API_KEYenv+echo lines (action.yml:48, 51), and theonesignal-api-key:line ine2e.yml:32; orexamples/demo/app/build.gradle.ktsby addingdemoOverride("ONESIGNAL_API_KEY")and exposing it viabuildConfigField/resValueso the demo app (or a runtime test hook) can actually consume it.