Skip to content

fix: flaky test Request-queue UI changes handles three confirmations on three confirmations concurrently#25675

Merged
seaona merged 16 commits into
developfrom
flaky-test-fix-handle-windows-ui
Jul 5, 2024
Merged

fix: flaky test Request-queue UI changes handles three confirmations on three confirmations concurrently#25675
seaona merged 16 commits into
developfrom
flaky-test-fix-handle-windows-ui

Conversation

@seaona

@seaona seaona commented Jul 4, 2024

Copy link
Copy Markdown
Member

Description

The queue UI test fails has a couple of flaky points.

Sometimes we see the console errors for network unresponsive -- when this happens the test fails.
When we don't see the console errors for network unresponsive the test passes. The problem is that in the tests we were ignoring the PollingBlock errors, but more errors appear when the network is down, making the test fail when they appear in the console.

Screenshot from 2024-07-05 14-45-37

https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/90602/workflows/e41e1e65-d445-4b60-9602-89a261cc5654/jobs/3362082/parallel-runs/4?filterBy=ALL

There are more sources of flakiness, that can also cause this test to fail. Those are described here and fixed in this same PR:

  • Performing a subsequent action when the popup is closed, causes the NoSuchWindowError: no such window: target window already closed error. We need to switch back to an existing window, before proceeding with test actions

  • Triggering a tx/signature with an unresponsive network (when we kill ganache) takes a significant amount of time to open the popup. Now we wait for the expected number of windows before switching to the popup and we add a big timeout for this case (it's a max value and won't delay the test if it's not needed, like in FF). There's an issue open to fix this on the app level in this ticket.

  • Asserting values from elements found by their css/testid can introduce flakiness as the elements could be rendered without having the expected text yet loaded. Now the approach for these validations has been fixed by looking for the exact selector which includes the text

  • A lavamoat error appeared once, but this seem unrelated to this specific test and just saw it once so it's left outside the scope of this PR error caught in retry(): JavascriptError: javascript error: LavaMoat - property "name" of globalThis is inaccessible under scuttling mode. To learn more visit https://github.com/LavaMoat/LavaMoat/pull/360.

  • Failure 2:

  • Logs:

[driver] Called 'switchToWindowWithTitle' with arguments ["MetaMask Dialog",["8C425F6EC00FF98E5C0A643C0CDF0DD6","0BF6602327E4E63F0FAE641DF2BB71E0","45802EBF75EF61BE99D7F235354A0FC6","109C90EB942C664573D051FD7952C0CE","4199B96477AFE13BB203FFC80C399B9F"]]
[driver] Called 'findClickableElement' with arguments ["[data-testid=\"confirmation-submit-button\"]"]
[driver] Called 'clickElement' with arguments ["[data-testid=\"confirmation-submit-button\"]"]
[driver] Called 'openNewPage' with arguments ["http://127.0.0.1:8082"]
Failed to handle request: socket hang up
Failure on testcase: 'Request-queue UI changes handles three confirmations on three confirmations concurrently @no-mmi', for more information see the artifacts tab in CI

NoSuchWindowError: no such window: target window already closed
from unknown error: web view not found
JsonRpcEngine: Response has no error or result for request:
{
  "jsonrpc": "2.0",
  "id": "8871ca57-2969-45f2-8838-115e34bfa417",
  "method": "eth_blockNumber",
  "params": [],
  "origin": "metamask",
  "networkClientId": "networkConfigurationId",
  "tabId": 1870836432
}
  at m._fetchLatestBlock (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:357185)
  at async m._updateLatestBlock (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:356869)
  at async m._updateAndQueue (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:357352)
  at m._updateAndQueue (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:357401)

----------End of Chrome error----------
---This error is on the ignore list----


[driver] Called 'findElement' with arguments ["[data-testid=\"network-display\"], [data-testid=\"signature-request-network-display\"]"]
[driver] Called 'findElement' with arguments [".confirm-page-container-summary__origin bdi, .request-signature__origin .chip__label"]
[driver] Called 'executeScript' with arguments [null]
Failure on testcase: 'Request-queue UI changes should gracefully handle network connectivity failure for confirmations @no-mmi', for more information see the artifacts tab in CI
  • Failure 4: hasn't appeared but since it could potentially appear in the future, has also been fixed
failing-test-window-ui.mp4

Open in GitHub Codespaces

Related issues

Fixes: #25674

Manual testing steps

  1. Check ci

Screenshots/Recordings

Screenshot from 2024-07-04 09-03-08

Screenshot from 2024-07-05 10-03-57

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.

@seaona seaona added team-extension-platform Extension Platform team flaky tests labels Jul 4, 2024
@seaona seaona self-assigned this Jul 4, 2024
@seaona seaona requested a review from a team as a code owner July 4, 2024 07:03
@github-actions

github-actions Bot commented Jul 4, 2024

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-confirmations Push issues to confirmations team label Jul 4, 2024
@seaona seaona marked this pull request as draft July 4, 2024 07:19
@sonarqubecloud

sonarqubecloud Bot commented Jul 4, 2024

Copy link
Copy Markdown

@seaona seaona closed this Jul 4, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 4, 2024
@seaona seaona reopened this Jul 5, 2024
@seaona seaona removed the team-confirmations Push issues to confirmations team label Jul 5, 2024
@seaona seaona marked this pull request as ready for review July 5, 2024 13:53
// Differs between confirmation and signature
'.confirm-page-container-summary__origin bdi, .request-signature__origin .chip__label',
);
const originText = await originElement.getText();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is a bad pattern that can cause flakiness: if we get the element without waiting for the exact text contents we want, it can be a race condition when the element is rendered but the value is not there yet, making all the assertions fail

@metamaskbot

Copy link
Copy Markdown
Collaborator
Builds ready [33d806b]
Page Load Metrics (286 ± 249 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7748915010249
domContentLoaded1199432512
load471911286518249
domInteractive1199432512
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

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

LGTM ! Thanks !

@seaona seaona merged commit 52f6ad3 into develop Jul 5, 2024
@seaona seaona deleted the flaky-test-fix-handle-windows-ui branch July 5, 2024 15:51
@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Jul 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

flaky tests release-12.2.0 Issue or pull request that will be included in release 12.2.0 team-extension-platform Extension Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flaky test: Request-queue UI changes handles three confirmations on three confirmations concurrently

4 participants