Skip to content

test: fixing flakiness#28662

Draft
javiergarciavera wants to merge 1 commit intomainfrom
MMQA-1701-rn-performance-tests
Draft

test: fixing flakiness#28662
javiergarciavera wants to merge 1 commit intomainfrom
MMQA-1701-rn-performance-tests

Conversation

@javiergarciavera
Copy link
Copy Markdown
Contributor

@javiergarciavera javiergarciavera commented Apr 10, 2026

Description

Changelog

CHANGELOG entry:

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Moderate risk because it changes Playwright project matching/build configuration and alters iOS-specific element lookup/polling logic, which can affect what runs in CI and how tests behave across platforms.

Overview
Improves homepage token section testability by adding TokensSectionTestIds and wiring testID into the TokensSection SectionHeader (including trending-only mode), then updating WalletView page objects to target that ID instead of header text.

Reduces iOS Appium/Playwright flakiness by switching some iOS lookups to accessibility-id selectors, updating token row selection similarly, and revising waitForBalanceToStabilize to first wait for the balance empty-state banner to disappear before running the usual stability polling.

Adjusts performance test expectations/config by increasing the asset-view timer thresholds and narrowing BrowserStack Playwright projects to only run asset-view.spec.ts, with hardcoded BrowserStack buildPath URLs.

Reviewed by Cursor Bugbot for commit a37b8b6. Bugbot is set up for automated code reviews on this repo. Configure here.

@javiergarciavera javiergarciavera requested a review from a team as a code owner April 10, 2026 13:18
@javiergarciavera javiergarciavera marked this pull request as draft April 10, 2026 13:18
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-qa QA team label Apr 10, 2026
@github-actions github-actions bot added size-M risk-medium Moderate testing recommended · Possible bug introduction risk labels Apr 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Selected Performance tags: @PerformanceAssetLoading, @PerformanceLogin, @PerformanceLaunch, @PerformanceOnboarding
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes in this PR are focused on performance test infrastructure and iOS selector reliability improvements, not on Detox E2E smoke test flows:

  1. TokensSection.testIds.ts (new file) + TokensSection.tsx: Adds a testID attribute to the SectionHeader component in the Tokens section. This is a purely additive, low-risk change that enables more reliable test selectors. No functional behavior changes.

  2. WalletView.ts (page object): Improves iOS selector reliability:

    • totalBalance and tokenRow() now use accessibility ID selectors on iOS (faster/more reliable)
    • tokensSection switches from text-based to ID-based selector using the new TokensSectionTestIds.SECTION_HEADER
    • waitForBalanceToStabilize() gets a major iOS improvement with empty-state banner detection before stability polling
      These are Playwright/Appium page object changes, not Detox. They don't affect Detox E2E smoke tests.
  3. asset-view.spec.ts: Increases performance threshold from 600ms to 1500ms for asset view timing. This is a performance test spec change only.

  4. playwright.config.ts: Narrows BrowserStack test matching to only asset-view.spec.ts and hardcodes specific BrowserStack build URLs. This is CI/performance test infrastructure only.

None of these changes affect Detox E2E smoke test flows. The WalletView.ts page object is used by Playwright performance tests, not Detox smoke tests. No smoke test tags are warranted.

For performance tests: The changes directly affect @PerformanceAssetLoading (asset-view.spec.ts threshold change) and @PerformanceLogin (same spec uses both tags). The WalletView.ts selector improvements also affect tests using tokensSection, totalBalance, and tokenRow which span @PerformanceLaunch (warm-start tests use totalBalance) and @PerformanceOnboarding (import-wallet.spec.ts uses tokensSection/tokenRow). Running these performance tests validates the selector changes work correctly and the new thresholds are appropriate.

Performance Test Selection:
Performance tests should run because: (1) asset-view.spec.ts directly changes the @PerformanceAssetLoading + @PerformanceLogin threshold from 600ms to 1500ms - these tests must run to validate the new threshold is correct and the test passes; (2) WalletView.ts selector changes (totalBalance, tokenRow, tokensSection) affect multiple performance specs - warm-start-login-to-wallet.spec.ts uses totalBalance (@PerformanceLaunch), import-wallet.spec.ts uses tokensSection/tokenRow (@PerformanceOnboarding); (3) playwright.config.ts narrows BrowserStack scope to asset-view.spec.ts specifically, confirming this is the primary test being validated; (4) The iOS waitForBalanceToStabilize() improvement affects all performance tests that call this method including asset-balances.spec.ts and launch-times specs.

View GitHub Actions results

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a37b8b6. Configure here.

<SectionHeader
title={title}
onPress={handleViewAllTokens}
testID={TokensSectionTestIds.SECTION_HEADER}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Duplicate testID on simultaneously rendered sections

High Severity

Both TokensSectionMain and TokensSectionTrendingOnly use the identical TokensSectionTestIds.SECTION_HEADER ('tokens-section-header') as their testID. In Homepage.tsx, when separateTrending is true, both components render simultaneously on-screen (lines ~234 and ~290). This produces two DOM elements with the same testID, making the WalletView.tokensSection E2E selector ambiguous — it may match the trending section instead of the main tokens section. The PR aims to fix flakiness but this introduces a new source of it.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a37b8b6. Configure here.

launchableActivity: 'io.metamask.MainActivity',
},
buildPath: process.env.BROWSERSTACK_ANDROID_APP_URL, // Path to Browserstack url
buildPath: 'bs://c0fac532d9c0e9db7e7031931029b09564307577', // Path to Browserstack url
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hardcoded BrowserStack URLs replace environment variables

High Severity

The buildPath for browserstack-android and browserstack-ios projects was changed from process.env.BROWSERSTACK_ANDROID_APP_URL / process.env.BROWSERSTACK_IOS_APP_URL to hardcoded BrowserStack hashes (bs://c0fac53... and bs://13447ba...). These point to a specific build artifact and will fail for all other CI runs or team members. Other projects in the same config (e.g., mm-connect-android-browserstack) still correctly use environment variables.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a37b8b6. Configure here.

// Browserstack does not support appium 3 just yet.
name: 'browserstack-android',
testMatch: '**/performance/login/**/*.spec.ts',
testMatch: '**/performance/login/**/asset-view.spec.ts',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BrowserStack testMatch narrowed to single file only

Medium Severity

The testMatch for browserstack-android and browserstack-ios projects changed from **/performance/login/**/*.spec.ts to **/performance/login/**/asset-view.spec.ts. This excludes at least 7 other performance test files (e.g., asset-balances.spec.ts, cross-chain-swap-flow.spec.ts, eth-swap-flow.spec.ts) from running on BrowserStack. The local android and ios projects still use the wildcard pattern, suggesting this narrowing was a debugging change that got committed.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a37b8b6. Configure here.

@sonarqubecloud
Copy link
Copy Markdown

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

Labels

risk-medium Moderate testing recommended · Possible bug introduction risk size-M team-qa QA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants