Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: flaky test Request Queuing for Multiple Dapps and Txs on different networks should batch confirmation txs for different dapps on different networks. #27095

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

seaona
Copy link
Contributor

@seaona seaona commented Sep 12, 2024

Description

In this test, there is an anti-pattern, where we first find an element and then assert its inner value. This is a source of flakiness, since at the moment we assert the value, it could be that it doesn't have the expected value yet, making the assertion to fail.

let navigationElement = await driver.findElement(
  '.confirm-page-container-navigation',
);
let navigationText = await navigationElement.getText();
assert.equal(navigationText.includes('1 of 2'), true);

Screenshot from 2024-09-12 10-55-08

Instead of this, we should try to directly find the element by its value. This ensures that we wait until the element has the right value.

Note: in this case, since the value is broken down, we find the value by the xpath.

Screenshot from 2024-09-12 10-57-15

Open in GitHub Codespaces

Related issues

Fixes: #26933

Manual testing steps

  1. Check ci

Screenshots/Recordings

Above

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 requested a review from a team as a code owner September 12, 2024 08:52
@seaona seaona self-assigned this Sep 12, 2024
Copy link
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.

@seaona seaona added Sev2-normal Normal severity; minor loss of service or inconvenience. area-qa Relating to QA work (Quality Assurance) labels Sep 12, 2024
Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [82ec20f]
Page Load Metrics (1728 ± 115 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31023431664386185
domContentLoaded144923201710227109
load145724041728240115
domInteractive12120392412
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@seaona seaona merged commit 5701a0b into develop Sep 12, 2024
115 of 116 checks passed
@seaona seaona deleted the fix-flaky-assert-queue branch September 12, 2024 11:17
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 12, 2024
@metamaskbot metamaskbot added release-12.5.0 Issue or pull request that will be included in release 12.5.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-qa Relating to QA work (Quality Assurance) release-12.5.0 Issue or pull request that will be included in release 12.5.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fix "Request Queuing for Multiple Dapps and Txs on differ..." flaky tests
5 participants