Skip to content

test: adds 2 proof of concepts to fetch logs from test dapps for mm-connect#27124

Draft
christopherferreira9 wants to merge 3 commits into
mainfrom
cferreira/enable-browserstack-logs
Draft

test: adds 2 proof of concepts to fetch logs from test dapps for mm-connect#27124
christopherferreira9 wants to merge 3 commits into
mainfrom
cferreira/enable-browserstack-logs

Conversation

@christopherferreira9
Copy link
Copy Markdown
Contributor

@christopherferreira9 christopherferreira9 commented Mar 6, 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
Test-only changes, but they modify the shared CommandQueueServer fixture (new routes/state and CORS preflight handling), which could affect existing E2E runs if assumptions about endpoints or in-memory state change.

Overview
Adds two new Appwright projects (chris-dummy and chris-dummy-server) that run new MM Connect dummy specs on BrowserStack Android using a fixed bs://... build.

Extends CommandQueueServer with CORS OPTIONS handling plus a simple in-memory MM Connect debug log collector (GET /mm-connect-debug.json, POST /mm-connect-debug) and a polling helper getMMConnectDebugLogs().

Introduces two POC tests: one injects a WebView console interceptor and prints captured logs, and another starts CommandQueueServer and fetches logs posted by the dapp.

Written by Cursor Bugbot for commit fae8de3. This will update automatically on new commits. Configure here.

@christopherferreira9 christopherferreira9 requested a review from a team as a code owner March 6, 2026 12:24
@christopherferreira9 christopherferreira9 marked this pull request as draft March 6, 2026 12:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

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 Mar 6, 2026
@christopherferreira9 christopherferreira9 added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Mar 6, 2026
@github-actions github-actions Bot added the size-M label Mar 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The changes are entirely within the test infrastructure:

  1. tests/appwright.config.ts: Adds two new test project configurations (chris-dummy and chris-dummy-server) for performance tests. These are additive configurations that don't affect existing test projects.

  2. tests/framework/fixtures/CommandQueueServer.ts: Adds new functionality for MM Connect debug logging:

    • New _mmConnectDebugLogs property
    • OPTIONS request handling for CORS preflight (additive, doesn't affect existing requests)
    • New GET /mm-connect-debug.json and POST /mm-connect-debug endpoints
    • New getMMConnectDebugLogs() method

    All changes are additive and backward compatible. Existing endpoints (/queue.json, /debug.json, /exported-state) remain unchanged.

  3. tests/performance/mm-connect/dummy-server.spec.js and tests/performance/mm-connect/dummy.spec.js: New dummy/experimental test files for performance testing development (named "chris-dummy" suggesting development work).

No app code is modified. The CommandQueueServer is used by some existing tests (perps, state-export), but the changes are purely additive - new endpoints and methods that don't modify existing behavior. The existing test flows won't be affected.

Since these are performance test infrastructure changes with no impact on Detox E2E tests or app functionality, no E2E test tags are needed.

Performance Test Selection:
While these changes are in the tests/performance directory and add new performance test infrastructure (dummy tests for MM Connect debug logging), the new tests are experimental/development tests (named 'chris-dummy') and are not part of the standard performance test suite. The changes add new test configurations and test files but don't modify existing performance tests or app code that would require running the existing performance test suite. The CommandQueueServer changes are additive infrastructure for future performance testing capabilities.

View GitHub Actions results

Copy link
Copy Markdown
Contributor

@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.

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

throw new Error(
`getMMConnectDebugLogs timed out after ${timeout}ms — the app did not POST to /mm-connect-debug.json. ` +
'Ensure the command queue server is running and the app is polling /mm-connect-debug.json.',
);
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.

Error message references wrong POST endpoint path

Medium Severity

The timeout error message in getMMConnectDebugLogs tells users the app did not POST to /mm-connect-debug.json, but _isMMConnectDebugLogPost defines the actual POST route as /mm-connect-debug (without .json). Someone debugging a timeout failure would be directed to the wrong endpoint. The existing getExportedState error correctly references /exported-state (the POST path), so this is inconsistent.

Additional Locations (1)

Fix in Cursor Fix in Web


test.afterAll(async () => {
// await commandQueueServer.stop();
});
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.

Server stop commented out causing resource leak

Low Severity

The commandQueueServer.stop() call is commented out in test.afterAll, so the Koa server started in beforeAll is never shut down. This leaks the server process and port 2446, which can cause port-in-use failures on subsequent test runs or in CI.

Fix in Cursor Fix in Web

Comment thread tests/appwright.config.ts
name: process.env.BROWSERSTACK_DEVICE || 'Samsung Galaxy S23 Ultra', // this can changed
osVersion: process.env.BROWSERSTACK_OS_VERSION || '13.0', // this can changed
},
buildPath: 'bs://a765924da41819f483f9b3143c3560f5e4a53e88', // Path to Browserstack url
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.

Hardcoded BrowserStack build hash will become stale

Medium Severity

Both new config entries hardcode buildPath: 'bs://a765924da41819f483f9b3143c3560f5e4a53e88' — a specific BrowserStack build hash. Unlike other configs that use 'PATH-TO-BUILD' as a placeholder, this is a concrete value that will become stale as builds expire. Other entries in this config use environment variables or explicit placeholders for the build path, so this breaks the established pattern.

Additional Locations (1)

Fix in Cursor Fix in Web

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 6, 2026

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

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed size-M team-qa QA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants