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 tests Queued Confirmations... in FF for unordered events #30031

Merged
merged 14 commits into from
Jan 31, 2025

Conversation

seaona
Copy link
Contributor

@seaona seaona commented Jan 31, 2025

Description

An unordered metric event makes the test fail, as we see the signature one (performed after in 8081) instead of the tx one (performed first in 8080)

Screenshot from 2025-01-31 17-28-34

After looking at the logs, we can see how the 8081 event refers to the signature event that happens after the transaction

image

Furthermore, the wait for chainID added here was pointing to the old chainId, so now it's corrected and fixes remaining flakiness

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

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.

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.

@metamaskbot metamaskbot added the team-qa QA team label Jan 31, 2025
@seaona seaona changed the title fix: new flakiness on queued fix: new flakiness on queued in FF Jan 31, 2025
@@ -312,7 +312,7 @@ describe('Queued Confirmations', function () {
// create deposit transaction in dapp 1
await createDepositTransaction(driver);

await driver.delay(2000);
await driver.delay(5000);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the buffer time needs to be a bit bigger otherwise the metric events get unordered, and we get first the signature event from the dapp2 (localhost:8081) and the asserts fail

@@ -420,7 +420,7 @@ async function switchChainToDappOne(driver: Driver) {
// No dialog should appear as we already gave permissions to this network
await driver.waitForSelector({
css: '[id="chainId"]',
text: '0x539',
text: '0x3e8',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

my mistake, we should wait for the new chain id (changed 2 steps above)

@seaona seaona marked this pull request as ready for review January 31, 2025 16:21
@seaona seaona changed the title fix: new flakiness on queued in FF fix: new flakiness on queued in FF for unordered events Jan 31, 2025
@seaona seaona self-assigned this Jan 31, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [ba5f69e]
Page Load Metrics (1567 ± 83 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint13952195156717283
domContentLoaded13572185154917383
load13902192156717383
domInteractive2294362210
backgroundConnect85620147
firstReactRender1675382412
getState410621
initialActions01000
loadScripts9561636111014570
setupStore76218199
uiStartup15722450175918589
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link
Contributor

@desi desi left a comment

Choose a reason for hiding this comment

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

LGTM

@seaona seaona enabled auto-merge January 31, 2025 18:27
@seaona seaona added this pull request to the merge queue Jan 31, 2025
@DDDDDanica
Copy link
Contributor

LGTM !

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 31, 2025
@desi desi added this pull request to the merge queue Jan 31, 2025
Merged via the queue into main with commit c75493d Jan 31, 2025
71 checks passed
@desi desi deleted the fix-flakiness-queued-2 branch January 31, 2025 21:50
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2025
@metamaskbot metamaskbot added the release-12.13.0 Issue or pull request that will be included in release 12.13.0 label Jan 31, 2025
@seaona seaona changed the title fix: new flakiness on queued in FF for unordered events fix: flaky tests Queued Confirmations... in FF for unordered events Feb 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.13.0 Issue or pull request that will be included in release 12.13.0 team-qa QA team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants