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.

Sign up for GitHub

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

Jump to bottom

Fix "Dapp viewed Event @no-mmi is sent when refreshing da..." flaky tests #26899

Closed
hjetpoluru opened this issue Sep 4, 2024 · 2 comments
Closed

Fix "Dapp viewed Event @no-mmi is sent when refreshing da..." flaky tests #26899

hjetpoluru opened this issue Sep 4, 2024 · 2 comments
Assignees
@hjetpoluru
Labels
flaky tests Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug

Comments

@hjetpoluru
Copy link
Contributor

hjetpoluru commented Sep 4, 2024 •
edited
Loading

CI link for the flaky test reported on Sept 26
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/102230/workflows/464f7ef8-7df3-4ed9-80c9-776e71b1297f/jobs/3808841/tests

CI link for the flaky test reported on Sept 24
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/102034/workflows/87cdde9f-c174-4463-9c1f-d7ac54603e67/jobs/3800442/tests

https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/98663/workflows/e99246eb-772f-4339-aacb-b3ebe039e54f/jobs/3672023/tests

Test scenario - Dapp viewed Event @no-mmi is sent when refreshing dapp with one account connected
Build - webpack
Browser - Chrome
Test path - test/e2e/tests/metrics/dapp-viewed.spec.js

Expected values to be strictly equal:
+ actual - expected

+ undefined
- false
  (Ran on CircleCI Node 12 of 20, Job test-e2e-chrome-webpack)
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ undefined
- false
    at /home/circleci/project/test/e2e/tests/metrics/dapp-viewed.spec.js:197:16
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async withFixtures (test/e2e/helpers.js:212:5)
    at async Context.<anonymous> (test/e2e/tests/metrics/dapp-viewed.spec.js:172:5)

      + expected - actual:  failed to generate Mocha diff

image

image
@hjetpoluru hjetpoluru added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform flaky tests labels Sep 4, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Sep 4, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Sep 19, 2024
@hjetpoluru hjetpoluru self-assigned this Sep 24, 2024
@hjetpoluru hjetpoluru mentioned this issue Sep 25, 2024
Merged
7 tasks
@chloeYue
Copy link
Contributor

this test is still flaky.
CI failure: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/102346/workflows/3d79e5dc-c1ea-4418-acc6-4601394f647e/jobs/3813643

This was referenced Sep 27, 2024
Closed
Closed
github-merge-queue bot pushed a commit that referenced this issue Oct 4, 2024
@hjetpoluru
fix: "Dapp viewed Event @no-mmi is sent when refreshing da..." flaky …
9f68d10 
…test (#27381)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

In this test, there are two different requests for the "Dapp Viewed"
event: one with the property is_first_visit: true and the other with
is_first_visit: false. However, the current mock setup does not
differentiate between these two requests.
To ensure that both "Dapp Viewed" event requests are properly handled,
we need to create two separate mocks has been created.

Special thanks to @seaona for her thorough analysis and understanding of
this tricky flaky test. Her insights and proposed solution were
instrumental in resolving this issue. All credit for identifying and
addressing this problem goes to her.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27381?quickstart=1)

## **Related issues**

Fixes:
#24655
#24651
#26899

## **Manual testing steps**

Run the dapp viewed spec locally or in codespace using below commands
against chrome browser:
yarn
yarn build:test:webpack
ENABLE_MV3=false yarn test:e2e:single
test/e2e/tests/metrics/dapp-viewed.spec.js --browser=chrome

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
@hjetpoluru
Copy link
Contributor Author

Fix in PR 27381

@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Oct 4, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hjetpoluru hjetpoluru

Labels
flaky tests Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug
Projects
Bugs by severity
Archived in project
Milestone
No milestone
Development

No branches or pull requests
2 participants
@chloeYue @hjetpoluru