Skip to content

Commit

Permalink
fix: "Dapp viewed Event @no-mmi is sent when refreshing da..." flaky …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
hjetpoluru authored Oct 4, 2024
1 parent c805f75 commit 9f68d10
Showing 1 changed file with 45 additions and 17 deletions.
62 changes: 45 additions & 17 deletions test/e2e/tests/metrics/dapp-viewed.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,40 @@ const {
MetaMetricsEventName,
} = require('../../../../shared/constants/metametrics');

async function mockedDappViewedEndpoint(mockServer) {
async function mockedDappViewedEndpointFirstVisit(mockServer) {
return await mockServer
.forPost('https://api.segment.io/v1/batch')
.withJsonBodyIncluding({
batch: [{ type: 'track', event: MetaMetricsEventName.DappViewed }],
batch: [
{
type: 'track',
event: MetaMetricsEventName.DappViewed,
properties: {
is_first_visit: true,
},
},
],
})
.thenCallback(() => {
return {
statusCode: 200,
};
});
}

async function mockedDappViewedEndpointReVisit(mockServer) {
return await mockServer
.forPost('https://api.segment.io/v1/batch')
.withJsonBodyIncluding({
batch: [
{
type: 'track',
event: MetaMetricsEventName.DappViewed,
properties: {
is_first_visit: false,
},
},
],
})
.thenCallback(() => {
return {
Expand Down Expand Up @@ -67,7 +96,7 @@ describe('Dapp viewed Event @no-mmi', function () {
const validFakeMetricsId = 'fake-metrics-fd20';
it('is not sent when metametrics ID is not valid', async function () {
async function mockSegment(mockServer) {
return [await mockedDappViewedEndpoint(mockServer)];
return [await mockedDappViewedEndpointFirstVisit(mockServer)];
}

await withFixtures(
Expand All @@ -93,7 +122,7 @@ describe('Dapp viewed Event @no-mmi', function () {

it('is sent when navigating to dapp with no account connected', async function () {
async function mockSegment(mockServer) {
return [await mockedDappViewedEndpoint(mockServer)];
return [await mockedDappViewedEndpointFirstVisit(mockServer)];
}

await withFixtures(
Expand Down Expand Up @@ -125,8 +154,8 @@ describe('Dapp viewed Event @no-mmi', function () {
it('is sent when opening the dapp in a new tab with one account connected', async function () {
async function mockSegment(mockServer) {
return [
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpointFirstVisit(mockServer),
await mockedDappViewedEndpointReVisit(mockServer),
await mockPermissionApprovedEndpoint(mockServer),
];
}
Expand Down Expand Up @@ -163,8 +192,8 @@ describe('Dapp viewed Event @no-mmi', function () {
it('is sent when refreshing dapp with one account connected', async function () {
async function mockSegment(mockServer) {
return [
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpointFirstVisit(mockServer),
await mockedDappViewedEndpointReVisit(mockServer),
await mockPermissionApprovedEndpoint(mockServer),
];
}
Expand All @@ -189,10 +218,9 @@ describe('Dapp viewed Event @no-mmi', function () {
// refresh dapp
await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
await driver.refresh();

const events = await getEventPayloads(driver, mockedEndpoints);

// events are original dapp viewed, new dapp viewed when refresh, and permission approved
// events are original dapp viewed, navigate to dapp, new dapp viewed when refresh, new dapp viewed when navigate and permission approved
const dappViewedEventProperties = events[1].properties;
assert.equal(dappViewedEventProperties.is_first_visit, false);
assert.equal(dappViewedEventProperties.number_of_accounts, 1);
Expand All @@ -204,10 +232,10 @@ describe('Dapp viewed Event @no-mmi', function () {
it('is sent when navigating to a connected dapp', async function () {
async function mockSegment(mockServer) {
return [
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpointFirstVisit(mockServer),
await mockedDappViewedEndpointReVisit(mockServer),
await mockedDappViewedEndpointFirstVisit(mockServer),
await mockedDappViewedEndpointReVisit(mockServer),
await mockPermissionApprovedEndpoint(mockServer),
];
}
Expand Down Expand Up @@ -247,7 +275,7 @@ describe('Dapp viewed Event @no-mmi', function () {

it('is sent when connecting dapp with two accounts', async function () {
async function mockSegment(mockServer) {
return [await mockedDappViewedEndpoint(mockServer)];
return [await mockedDappViewedEndpointFirstVisit(mockServer)];
}
await withFixtures(
{
Expand Down Expand Up @@ -299,8 +327,8 @@ describe('Dapp viewed Event @no-mmi', function () {
it('is sent when reconnect to a dapp that has been connected before', async function () {
async function mockSegment(mockServer) {
return [
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpoint(mockServer),
await mockedDappViewedEndpointFirstVisit(mockServer),
await mockedDappViewedEndpointReVisit(mockServer),
];
}

Expand Down

0 comments on commit 9f68d10

Please sign in to comment.