Skip to content

feat(perf): auto-enable inline app profiling on failed PR performance tests - #33902

Merged
javiergarciavera merged 10 commits into
mainfrom
MMQA-enable-app-profiling
Jul 29, 2026
Merged

feat(perf): auto-enable inline app profiling on failed PR performance tests#33902
javiergarciavera merged 10 commits into
mainfrom
MMQA-enable-app-profiling

Conversation

@javiergarciavera

@javiergarciavera javiergarciavera commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

Failed PR performance runs previously posted two comments: Performance Test Results (with copy/paste @metamaskbot commands) and a separate long App Profiling Check comment. That made PRs noisy and hard to scan.

This change embeds app profiling into the single Performance Test Results comment:

  1. Under each failed scenario with a usable prior baseline on main, show an inline App profiling check (short Summary + collapsed full metric table)
  2. Use ## ⚠️ Performance Test Results when there are failures ( when all green)
  3. Prefer last green baseline; fall back to latest usable profiling on main when the scenario is also red, labeled clearly
  4. Omit profiling blocks when there is no prior baseline
  5. Stop auto-posting a separate app-profiling comment (manual @metamaskbot / workflow_dispatch still works)
  6. On each PR update, delete previous <!-- perf-test-results --> and legacy <!-- app-profiling-check --> comments

Changelog

CHANGELOG entry: null

Related issues

Refs: MMQA-2081 (follow-up: auto-enable inline app profiling on failed PR performance scenarios)

Manual testing steps

Feature: Inline app profiling on failed PR performance tests

  Scenario: failed performance scenarios include profiling inline
    Given a PR triggers the performance E2E workflow with at least one failed scenario
    And aggregated-reports were uploaded for that run
    When the "Post Performance Results to PR" job finishes
    Then one Performance Test Results comment is posted
    And the header uses ⚠️ when there are failures
    And each failed scenario with a prior baseline shows App profiling check + Summary + collapsed metric table
    And no separate App Profiling Check comment is posted automatically

  Scenario: chronically red scenarios still get a baseline comparison
    Given a failed scenario that is also failing on main
    When the performance comment is enriched
    Then it uses the latest usable profilingSummary from main as baseline
    And the block labels the baseline as "scenario also failing"

  Scenario: scenarios without prior baseline are omitted from profiling
    Given a failed scenario that never ran with profiling on main
    When the performance comment is generated
    Then that scenario has no App profiling check block

  Scenario: all-green performance run stays compact
    Given a PR performance run with zero failed scenarios
    When the "Post Performance Results to PR" job finishes
    Then the header uses ⚡ and there is no failed-tests / profiling section

  Scenario: manual re-run still works
    Given a PR with a known performance run id
    When a reviewer posts `@metamaskbot app-profiling-check --all --run <RUN_ID>`
    Then the App Profiling Check workflow runs and posts/updates a standalone diff comment

Local unit verification:

node --test tests/scripts/diff-app-profiling.test.mjs

Screenshots/Recordings

N/A — CI/workflow and Node script changes only; no app UI. Evidence is the unified Performance Test Results PR comment produced by run-performance-e2e.yml.

Before

Two comments: Performance results (bot command column) + a separate long App Profiling Check comment.

After

One Performance Test Results comment with ⚠️ on failures and compact inline profiling under each failed scenario.

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
    • N/A for this PR: no app/runtime UI changes (CI comment + Node scripts only)
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
    • N/A for this PR: no app/runtime UI changes (CI comment + Node scripts only)
  • I've instrumented key operations with Sentry traces for production performance metrics
    • See trace() for usage and addToken for an example
    • N/A for this PR: no app/runtime UI changes (CI comment + Node scripts only)

For performance guidelines and tooling, see the Performance Guide.

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.
Open in Web Open in Cursor 

When a PR performance run has failed tests, the post-PR-comment job now
runs diff-app-profiling.mjs --all automatically and posts the baseline
diff table. Manual @metamaskbot re-runs remain available.

Adds --current-dir (reuse local aggregated-reports) and --replace
(refresh previous profiling comments).

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
@metamask-ci metamask-ci Bot added team-qa QA team INVALID-PR-TEMPLATE PR's body doesn't match template labels Jul 28, 2026
If a scenario never goes green on main, still compare against the latest
usable profilingSummary and label the baseline as also failing, so PR
diffs remain useful.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
@metamask-ci metamask-ci Bot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 28, 2026
@javiergarciavera
javiergarciavera marked this pull request as ready for review July 28, 2026 10:23
@javiergarciavera
javiergarciavera requested review from a team as code owners July 28, 2026 10:23
@javiergarciavera javiergarciavera added run-performance-tests Trigger performance tests workflow no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed labels Jul 28, 2026
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jul 28, 2026
Comment thread tests/scripts/diff-app-profiling.mjs
cursoragent and others added 2 commits July 28, 2026 10:46
gh api without --paginate only returns the first page of issue comments
(oldest-first), so recent app-profiling-check comments were missed and
duplicate diffs stacked on busy PRs. Also paginate perf-results cleanup.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
@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.

cursoragent and others added 2 commits July 28, 2026 11:58
When comparing multiple failed scenarios, the second download into the
same baseline-* dir failed with "file exists", so later scenarios (e.g.
Fresh SRP) incorrectly reported no usable baseline even though profiling
existed on main.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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 1 potential issue.

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 4f58459. Configure here.

Comment thread tests/scripts/diff-app-profiling.test.mjs
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 28, 2026
Export the helper with an injectable runGhFn so unit tests assert the
early-return reuse path and the download path without calling gh.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
racitores
racitores previously approved these changes Jul 28, 2026
@racitores racitores added the skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. label Jul 28, 2026
Show a short regression summary by default and move the full metric
table, variance disclaimer, and raw JSON into expandable details.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
Only include scenarios that have both usable current profiling and a
comparable baseline on main. Chronically new/unprofiled failures are
skipped so the PR comment lists only comparable regressions.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
@github-actions github-actions Bot added size-L and removed size-M labels Jul 28, 2026
Merge the separate App Profiling Check comment into the Performance
results comment: short summary + collapsed metric table per failed
scenario, ⚠️ header when tests fail, and drop the auto separate post.

Co-authored-by: javiergarciavera <javiergarciavera@users.noreply.github.com>
@cursor cursor Bot changed the title feat(perf): auto-run app profiling check on failed PR scenarios (MMQA) feat(perf): auto-enable inline app profiling on failed PR performance tests Jul 29, 2026
@metamask-ci metamask-ci Bot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 29, 2026
@github-actions github-actions Bot added size-XL and removed size-L labels Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 95%
click to see 🤖 AI reasoning details

E2E Test Selection:
All 6 changed files are purely CI/performance infrastructure with no app source code changes:

  1. .github/workflows/app-profiling-check.yml - Comment-only update (cosmetic: "Automatic" vs "Manual trigger" description)
  2. .github/workflows/run-performance-e2e.yml - CI workflow: adds GH_TOKEN env vars, updates comment deletion to paginate and also clean app-profiling-check comments
  3. tests/performance/README.md - Documentation update only
  4. tests/scripts/diff-app-profiling.mjs - Performance reporting script: adds --current-dir/--replace flags, non-green baseline fallback logic, embedded profiling sections in PR comments
  5. tests/scripts/diff-app-profiling.test.mjs - Unit tests for the above script changes
  6. tests/scripts/generate-performance-pr-comment.mjs - Performance PR comment generator: now enriches failed scenarios with inline app profiling comparisons

None of these changes touch any app code, E2E smoke test specs, page objects, selectors, flows, or shared test infrastructure that smoke tests depend on. The changes are entirely about how performance test results are reported in PR comments. No E2E smoke tags are warranted.

Performance Test Selection:
No performance spec files were changed. The changes are to performance reporting scripts (diff-app-profiling.mjs, generate-performance-pr-comment.mjs) and CI workflow configuration (run-performance-e2e.yml, app-profiling-check.yml). These scripts generate PR comments and compare profiling data — they do not measure or affect app performance metrics themselves. No performance test tags should be selected as the changes don't affect any performance scenario execution or measurement logic.

View GitHub Actions results

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

4 tests failed · 21 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

❌ Failed Tests (4)

🔬 App profiling vs main is included under each failed scenario that has a prior baseline.

@swap-bridge-dev-team

Cross-chain swap flow - ETH to SOL - 50+ accounts, SRP 1 + SRP 2 + SRP 3

Platform Device Reason Recording
Android Google Pixel 8 Pro (v14.0) Quality gates exceeded 📹 Watch

🔬 App profiling check · Current run 30438062320 · Baseline (last green on main) run 30431128018 @ c1faefc

Summary: ⚠️ 1 metric over +10%: CPU avg (+1.46 (+16.4%))

ℹ️ API calls unavailable: Network logs API error: Bad Request

Full metric table (+10% variance rules)

Disclaimer — allowed variance: a +10% margin over the baseline is permitted.

  • If Current <= Baseline + 10%, treated as acceptable noise.
  • If Current > Baseline + 10%, Current and variance % are highlighted with ⚠️.
Metric Baseline Current Δ
CPU avg 8.91% 10.37% +1.46 (+16.4%) ⚠️
CPU max 24.37% 22.33% -2.04 (-8.4%)
Memory avg 713.14 MB 718.37 MB +5.23 (+0.7%)
Memory max 834.7 MB 828.83 MB -5.87 (-0.7%)
Slow frames 25.22% 18.9% -6.32 (-25.1%)
Frozen frames 0% 0% 0 (0%)
ANRs 0 0 0 (0%)
Issues 3 2 -1 (-33.3%)
Critical issues 2 2 0 (0%)
App size 318.33 MB 318.33 MB 0 (0%)

@Accounts-team

Import SRP with +50 accounts, SRP 1, SRP 2, SRP 3

Platform Device Reason Recording
Android Google Pixel 8 Pro (v14.0) Test error 📹 Watch

🔬 App profiling check · Current run 30438062320 · Baseline (last run on main (scenario also failing)) run 30435630458 @ 96deb2a

⚠️ No green baseline on main — comparing against the latest usable profiling.

Summary: ⚠️ 1 metric over +10%: Memory max (+117.09 (+15.5%))

ℹ️ API calls unavailable: Network logs API error: Bad Request

Full metric table (+10% variance rules)

Disclaimer — allowed variance: a +10% margin over the baseline is permitted.

  • If Current <= Baseline + 10%, treated as acceptable noise.
  • If Current > Baseline + 10%, Current and variance % are highlighted with ⚠️.
Metric Baseline Current Δ
CPU avg 7.37% 5.16% -2.21 (-30%)
CPU max 24.7% 23.61% -1.09 (-4.4%)
Memory avg 709.13 MB 726.98 MB +17.85 (+2.5%)
Memory max 753.72 MB 870.81 MB +117.09 (+15.5%) ⚠️
Slow frames 31.13% 32.03% +0.9 (+2.9%)
Frozen frames 0% 0% 0 (0%)
ANRs 0 0 0 (0%)
Issues 3 3 0 (0%)
Critical issues 2 2 0 (0%)
App size 318.33 MB 318.33 MB 0 (0%)

@mm-perps-engineering-team

Perps add funds

Platform Device Reason Recording
Android Google Pixel 8 Pro (v14.0) Quality gates exceeded 📹 Watch

🔬 App profiling check · Current run 30438062320 · Baseline (last green on main) run 30435630458 @ 96deb2a

Summary: ⚠️ 1 metric over +10%: CPU max (+3.52 (+14.9%))

ℹ️ API calls unavailable: Network logs API error: Bad Request

Full metric table (+10% variance rules)

Disclaimer — allowed variance: a +10% margin over the baseline is permitted.

  • If Current <= Baseline + 10%, treated as acceptable noise.
  • If Current > Baseline + 10%, Current and variance % are highlighted with ⚠️.
Metric Baseline Current Δ
CPU avg 11.16% 10.73% -0.43 (-3.8%)
CPU max 23.7% 27.22% +3.52 (+14.9%) ⚠️
Memory avg 702.57 MB 711.15 MB +8.58 (+1.2%)
Memory max 805.65 MB 809.14 MB +3.49 (+0.4%)
Slow frames 31.29% 29.3% -1.99 (-6.4%)
Frozen frames 0% 0% 0 (0%)
ANRs 0 0 0 (0%)
Issues 3 3 0 (0%)
Critical issues 2 1 -1 (-50%)
App size 318.33 MB 318.33 MB 0 (0%)

@metamask-onboarding-team

Fresh SRP wallet creation performance

Platform Device Reason Recording
Android Google Pixel 8 Pro (v14.0) Quality gates exceeded 📹 Watch

🔬 App profiling check · Current run 30438062320 · Baseline (last green on main) run 30429396115 @ 6b016f4

Summary: ⚠️ 2 metrics over +10%: Slow frames (+31.16 (+566.6%)), Issues (+1 (+50%))

ℹ️ API calls unavailable: Network logs API error: Bad Request

Full metric table (+10% variance rules)

Disclaimer — allowed variance: a +10% margin over the baseline is permitted.

  • If Current <= Baseline + 10%, treated as acceptable noise.
  • If Current > Baseline + 10%, Current and variance % are highlighted with ⚠️.
Metric Baseline Current Δ
CPU avg 11.07% 9.48% -1.59 (-14.4%)
CPU max 21.09% 21.93% +0.84 (+4%)
Memory avg 570.97 MB 572.35 MB +1.38 (+0.2%)
Memory max 855.31 MB 830.44 MB -24.87 (-2.9%)
Slow frames 5.5% 36.66% +31.16 (+566.6%) ⚠️
Frozen frames 0% 0% 0 (0%)
ANRs 0 0 0 (0%)
Issues 2 3 +1 (+50%) ⚠️
Critical issues 2 2 0 (0%)
App size 318.32 MB 318.33 MB +0.01 (+0%)
✅ Passed Tests (17)
Test Platform Device Duration Team Recording
Asset View, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 2.76s @assets-dev-team 📹 Watch
Aggregated Balance Loading Time, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 8.25s @assets-dev-team 📹 Watch
Swap flow - ETH to LINK, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 1.51s @swap-bridge-dev-team 📹 Watch
Cold Start: Measure ColdStart To Login Screen Android Google Pixel 8 Pro (v14.0) 4.01s @metamask-mobile-platform 📹 Watch
Measure Warm Start: Login To Wallet Screen Android Google Pixel 8 Pro (v14.0) 1.82s @metamask-mobile-platform 📹 Watch
Measure Warm Start: Warm Start to Login Screen Android Google Pixel 8 Pro (v14.0) 1.74s @metamask-mobile-platform 📹 Watch
Predict Available Balance - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 1.23s @team-predict 📹 Watch
Predict Deposit - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 11.73s @team-predict 📹 Watch
Predict Market Details - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 4.63s @team-predict 📹 Watch
Onboarding Import SRP with +50 accounts, SRP 3 Android Google Pixel 8 Pro (v14.0) 11.58s @metamask-onboarding-team 📹 Watch
Measure Cold Start To Onboarding Screen Android Google Pixel 8 Pro (v14.0) 4.23s @metamask-mobile-platform 📹 Watch
Cold Start after importing a wallet Android Google Pixel 8 Pro (v14.0) 1.50s @metamask-mobile-platform 📹 Watch
Account creation after fresh install Android Google Pixel 8 Pro (v14.0) 3.47s @metamask-onboarding-team 📹 Watch
Seedless Onboarding: Apple Login New User Android Google Pixel 8 Pro (v14.0) 10.93s @metamask-onboarding-team 📹 Watch
Perps open position and close it Android Google Pixel 8 Pro (v14.0) 17.33s @mm-perps-engineering-team 📹 Watch
Seedless Onboarding: Google Login New User Android Google Pixel 8 Pro (v14.0) 10.37s @metamask-onboarding-team 📹 Watch
Seedless Onboarding: Telegram Login New User Android Google Pixel 8 Pro (v14.0) 9.92s @metamask-onboarding-team 📹 Watch

Branch: MMQA-enable-app-profiling · Build: Normal · Commit: 09400eb · View full run

@metamask-ci metamask-ci Bot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 29, 2026
@javiergarciavera
javiergarciavera added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 7cefb5a Jul 29, 2026
103 of 106 checks passed
@javiergarciavera
javiergarciavera deleted the MMQA-enable-app-profiling branch July 29, 2026 12:22
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 29, 2026
@metamask-ci metamask-ci Bot added the release-8.6.0 Issue or pull request that will be included in release 8.6.0 label Jul 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-8.6.0 Issue or pull request that will be included in release 8.6.0 risk:medium AI analysis: medium risk run-performance-tests Trigger performance tests workflow size-XL skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-qa QA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants