ci: [SDK-4333] add appium E2E infrastructure with smoke test#27
ci: [SDK-4333] add appium E2E infrastructure with smoke test#27fadi-george merged 6 commits intomainfrom
Conversation
Add shared Appium testing setup for OneSignal SDK wrappers: - WebDriverIO configs for iOS/Android (local + BrowserStack) - Test helpers for app interaction, logging, and selectors - GitHub Actions composite action for CI integration - Smoke test placeholder to validate the pipeline Made-with: Cursor
Add unity, cordova, dotnet as valid SDK types. Rename ios-native -> ios, android-native -> android. Made-with: Cursor
Made-with: Cursor
e92c303 to
7f30e2b
Compare
Secrets (BrowserStack, OneSignal API key) and org vars (OneSignal app ID) are now accessed directly instead of being passed as inputs. The caller passes an artifact name + filename instead of a file path. Made-with: Cursor
bea56a0 to
1b21f7a
Compare
Move inputs.platform and inputs.app-filename to env vars in the
workflow to prevent expression injection via ${{ }} interpolation.
Fix byText() to use platform-specific visible text strategies
(iOS predicate string for label, UiSelector.text for Android)
instead of accessibility ID which doesn't match rendered text.
Made-with: Cursor
|
@claude review |
There was a problem hiding this comment.
The expression-injection fix and byText selector correction look good — thanks for addressing those. Remaining inline nits are minor and can be cleaned up before the follow-up PR adds real test callers.
Extended reasoning...
Overview
This PR introduces new Appium/WebDriverIO E2E infrastructure: platform configs, test helpers (app, logger, selectors), a shared wdio config with BrowserStack support, a GitHub Actions reusable workflow, and a trivial smoke test. No existing code is modified.
Security risks
A previous review flagged expression injection in the GitHub Actions workflow (inputs interpolated directly into a run: shell block). That issue has been resolved: inputs.platform is now passed via the env: block as PLATFORM and referenced as ${PLATFORM} in the shell script, which is the safe pattern. No remaining injection vectors are present in the workflow file.
Level of scrutiny
This is new CI test infrastructure, not production application code. The smoke test is expect(true).toBe(true). The helpers have no callers in this PR, so no runtime behavior changes land here. The nits flagged (error swallowing in deleteUser, default timeout in waitForLog, quote escaping in selectors) are real but only matter once the follow-up PR adds actual test callers.
Other factors
Both prior inline comments are resolved. The remaining three nit-level bugs are already posted as inline comments and should be addressed before the follow-up PR. The PR is appropriately scoped as part 1 of 2.
Summary
expect(true).toBe(true)) to validate the pipeline end-to-endDetails
Infrastructure added:
appium/directory with WebDriverIO setup for both local and BrowserStack runswdio.ios.conf.ts) and Android (wdio.android.conf.ts)wdio.shared.conf.ts) with BrowserStack integration, JUnit reporting, and test user cleanupapp.ts(app interaction),logger.ts(log polling),selectors.ts(cross-platform element selection).github/actions/appium-e2e/action.ymlcomposite action that handles app upload to BrowserStack, dependency install, test execution, and result artifact uploadWhy split into two PRs:
Keeps the infrastructure reviewable on its own and lets us validate the CI pipeline with a trivial test before layering on the real test suite.
Testing
Made with Cursor