Skip to content

feat: [SDK-4333] add shared Appium E2E composite action with BrowserStack#26

Open
fadi-george wants to merge 11 commits intomainfrom
fadi/sdk-4333-add-appium-sdk-shared-action
Open

feat: [SDK-4333] add shared Appium E2E composite action with BrowserStack#26
fadi-george wants to merge 11 commits intomainfrom
fadi/sdk-4333-add-appium-sdk-shared-action

Conversation

@fadi-george
Copy link
Copy Markdown
Contributor

Description

Add a reusable composite GitHub Action and shared WebdriverIO test suite for running Appium E2E tests against SDK demo apps via BrowserStack App Automate.

Details

Composite action (.github/actions/appium-e2e/action.yml):

  • Uploads app binary (APK/.app) to BrowserStack
  • Checks out sdk-shared to access shared test files
  • Installs WebdriverIO dependencies and runs tests against BrowserStack real devices
  • Uploads JUnit test results as artifacts
  • Inputs: platform, app-path, sdk-type, BrowserStack credentials, device config

WebdriverIO test suite (appium/):

  • Shared configs for Android and iOS with BrowserStack service integration
  • Local Appium fallback when BrowserStack credentials are absent (for local dev)
  • Cross-SDK selector abstraction supporting react-native, flutter, capacitor, ios-native, and android-native
  • Initial test specs: SDK init, login/logout, push subscription, tags
  • Test helpers for log view assertions and common app interactions

Selector strategy:

  • Native iOS (accessibilityIdentifier), Android Compose (testTag), RN (testID), Flutter (Semantics) all map to Appium accessibility ID
  • Capacitor uses data-testid CSS selectors in WebView context

Org secrets required: BROWSERSTACK_USERNAME, BROWSERSTACK_ACCESS_KEY, E2E_ONESIGNAL_APP_ID

Made with Cursor

@fadi-george fadi-george requested a review from a team as a code owner April 6, 2026 19:40
Comment on lines +54 to +58
case 'react-native':
case 'flutter':
case 'ios-native':
case 'android-native':
return $(`~${text}`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 In byText(), the capacitor case builds an XPath expression by directly interpolating the text argument into a double-quoted string literal: //*[contains(text(), "${text}")]. If text contains a double-quote character, the resulting XPath will be malformed and throw a runtime parse error. The fix is straightforward: either switch the XPath delimiter to single quotes (//*[contains(text(), '${text}')]) or use XPath concat() to safely handle both quote types.

Extended reasoning...

What the bug is and how it manifests

The byText() helper in selectors.ts handles the capacitor SDK type by constructing an XPath expression using a template literal: $(//*[contains(text(), "${text}")]). The text argument is interpolated directly into the XPath string without any escaping. If text contains a double-quote character ("), the resulting XPath string will have unbalanced quotes, making it syntactically invalid.

The specific code path that triggers it

At line 57 of appium/tests/helpers/selectors.ts:

case 'capacitor':
  return $(`//*[contains(text(), "${text}")]`);

Calling byText('say "hello"') produces the XPath string //*[contains(text(), "say "hello"")], where the embedded double quotes break the XPath string literal delimiters.

Why existing code doesn't prevent it

There is no input validation or escaping applied to text before it is interpolated. TypeScript's type system accepts any string value, so there is no compile-time guard against strings containing ".

What the impact would be

Any test that calls byText() for the capacitor SDK type with a string containing a double quote will throw a runtime XPath parse error, causing a confusing test failure that does not point to the root cause in the selector helper. This is test infrastructure, so the blast radius is limited to developer productivity rather than production correctness.

How to fix it

The simplest fix for strings that don't contain single quotes is to use single-quote XPath delimiters: //*[contains(text(), '${text}')]. For full safety against both quote types, use XPath concat(): //*[contains(text(), concat("'", '${text}', "'"))] is not right either. The correct fully-safe approach is: //*[contains(text(), concat("", '${text}', ""))] — or more practically, a small helper that splits the string on ' and builds a concat() expression.

Addressing the refutation

The refutation notes that all current callers use simple hardcoded strings like byText('Tags') and that UI labels in demo apps don't typically contain double quotes. This is true, and the risk is indeed low in practice. However, the bug is real: if any future test ever needs to match text containing a double quote (e.g. a JSON snippet, a formatted value, or a label with inch marks), the failure mode is a cryptic XPath parse error rather than a clean test failure. Since the fix is a one-line change, it is worth making.

Step-by-step proof

  1. SDK_TYPE=capacitor is set in the environment.
  2. Test calls byText('it\'s a "test"').
  3. getSdkType() returns 'capacitor'.
  4. The template literal evaluates to the string: //*[contains(text(), "it's a "test"")].
  5. The XPath parser sees contains(text(), "it's a " as a complete string argument, then encounters `test`` as an unexpected token.
  6. The Appium/WebDriver command throws a parse error, failing the test with no indication that the selector helper is at fault.


export const sharedConfig: WebdriverIO.Config = {
...(isLocal ? localConnection : browserstackConnection),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The specs field in wdio.shared.conf.ts is hardcoded to ['./tests/specs/tags.spec.ts'], so running npm run test:android or npm run test:ios locally will silently execute only the tags spec, skipping init, login-logout, and push-subscription. Fix by changing the value to ['./tests/specs/**/*.spec.ts'] to match the composite action's default glob.

Extended reasoning...

What the bug is and how it manifests

In appium/wdio.shared.conf.ts line 27, the specs array is hardcoded to ['./tests/specs/tags.spec.ts']. WebdriverIO uses this field as the default set of test files to run. Any invocation that does not explicitly override specs via the CLI --spec flag will silently run only tags.spec.ts, omitting the three other specs introduced in this very PR: init.spec.ts, login-logout.spec.ts, and push-subscription.spec.ts.

The specific code path that triggers it

The two npm scripts defined in package.json are:

"test:android": "wdio run wdio.android.conf.ts",
"test:ios":    "wdio run wdio.ios.conf.ts"

Neither passes --spec, so WebdriverIO falls through to sharedConfig.specs, which is the hardcoded single-file array.

Why existing code doesn't prevent it

The composite action in .github/actions/appium-e2e/action.yml always appends --spec '${{ inputs.test-spec }}' (defaulting to tests/specs/**/*.spec.ts) to the wdio invocation, which overrides the config. CI therefore runs all four specs and masks the misconfiguration. Local developers who follow the npm run path described in any onboarding or README will only ever run tags tests.

What the impact would be

A developer running npm run test:android locally would see only the tags spec execute and receive no indication that three other specs were skipped. Bugs in SDK initialization, login/logout, or push-subscription behaviour would go undetected in local runs, undermining the value of the test suite for local iteration.

How to fix it

Change line 27 in wdio.shared.conf.ts from:

specs: ['./tests/specs/tags.spec.ts'],

to:

specs: ['./tests/specs/**/*.spec.ts'],

This matches the composite action's default and ensures all specs run unless explicitly overridden.

Step-by-step proof

  1. Developer clones the repo and runs npm run test:android from appium/.
  2. wdio reads wdio.android.conf.ts, which spreads sharedConfig.
  3. sharedConfig.specs resolves to ['./tests/specs/tags.spec.ts'].
  4. No --spec flag is present on the command line, so WebdriverIO uses the config value as-is.
  5. Only tags.spec.ts runs; init.spec.ts, login-logout.spec.ts, and push-subscription.spec.ts are never loaded.
  6. The test run exits successfully with output referencing only the Tags suite, giving a false sense of complete coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant