Skip to content

test: improves websocket server teardown#30043

Merged
christopherferreira9 merged 10 commits into
mainfrom
cferreira/unblock-ci-1
May 13, 2026
Merged

test: improves websocket server teardown#30043
christopherferreira9 merged 10 commits into
mainfrom
cferreira/unblock-ci-1

Conversation

@christopherferreira9

@christopherferreira9 christopherferreira9 commented May 12, 2026 •
edited by cursor Bot
Loading

Copy link
Copy Markdown
Contributor

Description

This PR improves websocket server teardown to prevent port collision and skips failing tests.
Context: https://consensys.slack.com/archives/C02U025CVU4/p1778589879443169

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

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

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.

Note

Low Risk
Low risk: changes are limited to the E2E test harness and smoke specs, mainly improving WebSocket teardown/port release to avoid CI port collisions. Main downside is reduced coverage due to multiple smoke suites being temporarily skipped.

Overview
Improves E2E WebSocket teardown to avoid port collisions. LocalWebSocketServer now optionally tracks a ResourceType and always releases its PortManager allocation when stopping (including when the server was never started, and via a finally to guarantee cleanup on errors). withFixtures now constructs the account-activity WebSocket server with ResourceType.ACCOUNT_ACTIVITY_WS so this release happens.

Unblocks CI by skipping failing smoke tests. Several smoke specs (native send, EIP-7702 gas fee token flows, perps liquidation, and snap tests) are switched to describe.skip with comments pointing to the tracking thread.

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

@christopherferreira9 christopherferreira9 requested review from a team as code owners May 12, 2026 13:43
@github-actions

github-actions Bot commented May 12, 2026

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.

@github-actions github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 12, 2026
@metamaskbotv2 metamaskbotv2 Bot added the team-qa QA team label May 12, 2026
@christopherferreira9 christopherferreira9 removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 12, 2026
cursor[bot]
cursor Bot reviewed May 12, 2026
View reviewed changes
Comment thread tests/websocket/server.ts Outdated
Comment thread tests/smoke/confirmations/transactions/gas-fee-tokens-eip-7702.spec.ts Outdated
Comment thread tests/websocket/server.ts Outdated
@christopherferreira9 christopherferreira9 added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label May 12, 2026
cursor[bot]
cursor Bot reviewed May 12, 2026
View reviewed changes
Comment thread tests/smoke/confirmations/transactions/gas-fee-tokens-eip-7702.spec.ts
Comment thread tests/smoke/snaps/test-snap-get-entropy.spec.ts
cursor[bot]
cursor Bot reviewed May 12, 2026
View reviewed changes

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

There are 2 total unresolved issues (including 1 from previous review).

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 66ba1d9. Configure here.

Comment thread tests/smoke/confirmations/transactions/gas-fee-tokens-eip-7702.spec.ts Outdated
@christopherferreira9 christopherferreira9 requested a review from a team as a code owner May 12, 2026 14:23
racitores
racitores previously approved these changes May 12, 2026
View reviewed changes
@github-actions github-actions Bot added size-S and removed size-M labels May 12, 2026
@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations, SmokePerps, SmokeSnaps, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:

The PR makes two categories of changes:

  1. Test infrastructure fix (FixtureHelper.ts + server.ts): The LocalWebSocketServer now accepts an optional ResourceType parameter and properly releases port allocations via PortManager in both normal and error/timeout paths (using try/finally). The FixtureHelper.ts passes ResourceType.ACCOUNT_ACTIVITY_WS when creating the account activity WebSocket server. This is a stability fix to prevent stale port allocations from carrying over between tests. This infrastructure change affects any test that uses withFixtures with an account activity WebSocket server — notably the snaps entropy test and perps tests.

  2. Test skips: Multiple test suites are being skipped via describe.skip:

    • send-native-token.spec.ts (SmokeConfirmations) — moved to CV tests
    • gas-fee-tokens-eip-7702-sponsored.spec.ts (SmokeConfirmations) — CI instability
    • gas-fee-tokens-eip-7702.spec.ts (SmokeConfirmations) — CI instability
    • perps-position-liquidation.spec.ts (SmokePerps) — unblocking CI
    • test-snap-bip-44.spec.ts (SmokeSnaps) — skipped
    • test-snap-get-entropy.spec.ts (SmokeSnaps) — skipped (uses accountActivityWsServer)

Tag selection rationale:

  • SmokeConfirmations: Three test files in this suite are being skipped. Need to verify remaining tests in the suite still pass and the infrastructure change doesn't affect them.
  • SmokePerps: The perps liquidation test is skipped. Need to verify other perps tests still work. SmokePerps also requires SmokeWalletPlatform (Trending section) per tag description.
  • SmokeSnaps: Two snap tests are skipped (BIP-44 and get-entropy). The get-entropy test uses accountActivityWsServer which is directly affected by the infrastructure fix — need to verify the fix works correctly for remaining snap tests.
  • SmokeWalletPlatform: Required as a dependent tag when selecting SmokePerps per the tag description.

No performance tests are needed as these are test infrastructure/skip changes with no app code modifications.

Performance Test Selection:
No app source code was changed — all changes are in the test framework (WebSocket server port management) and test spec files (adding describe.skip). There is no impact on app rendering, data loading, startup, or any other performance-sensitive code path.

View GitHub Actions results

@sonarqubecloud

sonarqubecloud Bot commented May 12, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@christopherferreira9 christopherferreira9 added this pull request to the merge queue May 13, 2026
Merged via the queue into main with commit fef5dc4 May 13, 2026
108 checks passed
@christopherferreira9 christopherferreira9 deleted the cferreira/unblock-ci-1 branch May 13, 2026 07:15
@github-actions github-actions Bot locked and limited conversation to collaborators May 13, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.78.0 Issue or pull request that will be included in release 7.78.0 label May 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@FrederikBolding FrederikBolding FrederikBolding approved these changes

@jvbriones jvbriones jvbriones approved these changes

@gambinish gambinish gambinish approved these changes

@racitores racitores racitores left review comments

Assignees

No one assigned

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-7.78.0 Issue or pull request that will be included in release 7.78.0 size-S team-qa QA team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@christopherferreira9 @FrederikBolding @jvbriones @gambinish @racitores