Skip to content

Commit

Permalink
fix(e2e): Fixed flakiness of analytics tests (#17012)
Browse files Browse the repository at this point in the history
  • Loading branch information
@HajekOndrej @tomasklim
HajekOndrej authored and tomasklim committed Feb 18, 2025
1 parent 772e758 commit cdfe6d2
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 33 deletions.
14 changes: 12 additions & 2 deletions packages/suite-desktop-core/e2e/support/analytics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import { SuiteAnalyticsEvent } from '@trezor/suite-analytics';
import { EventPayload, Requests } from '@trezor/suite-web/e2e/support/types';

import { step } from './common';
import { expect } from '../support/fixtures';

export class AnalyticsFixture {
private page: Page;
private lastRequestCount = 0;
requests: Requests = [];

constructor(page: Page) {
Expand All @@ -25,8 +27,8 @@ export class AnalyticsFixture {
return event;
}

extractRequestTypes() {
return this.requests.map(request => request['c_type']);
findLatestRequestByType(eventType: SuiteAnalyticsEvent['type']) {
return [...this.requests].reverse().find(req => req.c_type === eventType);
}

//TODO: #15811 To be refactored
Expand All @@ -39,4 +41,12 @@ export class AnalyticsFixture {
route.continue();
});
}

@step()
async waitForAnalyticsRequests(expectedNewRequests = 1) {
await expect
.poll(() => this.requests.length)
.toBeGreaterThanOrEqual(this.lastRequestCount + expectedNewRequests);
this.lastRequestCount = this.requests.length;
}
}
8 changes: 4 additions & 4 deletions packages/suite-desktop-core/e2e/tests/analytics/events.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ test.describe('Analytics Events', { tag: ['@group=suite', '@webOnly'] }, () => {
// suite-ready is logged 1st, just check that it is reported when app is initialized and enabled
// device-connect is logged 2nd
// transport-type is logged 3rd
await analytics.waitForAnalyticsRequests(3);
expect(analytics.requests[0]).toHaveProperty('c_type', EventType.SuiteReady);
expect(analytics.requests[1]).toHaveProperty('c_type', EventType.DeviceConnect);
expect(analytics.requests[2]).toHaveProperty('c_type', EventType.TransportType);
expect(analytics.requests).toHaveLength(3);

const deviceConnectEvent = analytics.findAnalyticsEventByType<
ExtractByEventType<EventType.DeviceConnect>
Expand Down Expand Up @@ -74,8 +74,8 @@ test.describe('Analytics Events', { tag: ['@group=suite', '@webOnly'] }, () => {
// device-disconnect is logged 4th
await trezorUserEnvLink.stopEmu();

await expect.poll(() => analytics.requests).toHaveLength(4); // Poll to prevent race condition
expect(analytics.requests[3]).toHaveProperty('c_type', EventType.DeviceDisconnect);
await analytics.waitForAnalyticsRequests(1); // Poll to prevent race condition
expect(analytics.findLatestRequestByType(EventType.DeviceDisconnect)).toBeDefined();
});

test('reports suite-ready after enabling analytics on app initial run', async ({
Expand Down Expand Up @@ -137,7 +137,7 @@ test.describe('Analytics Events', { tag: ['@group=suite', '@webOnly'] }, () => {
await analyticsPage.continueButton.click();
await page.getByTestId('@onboarding/exit-app-button').click();

expect(analytics.requests.length).toBeGreaterThanOrEqual(2);
await analytics.waitForAnalyticsRequests(2);
expect(analytics.requests[0]).toHaveProperty('c_type', EventType.SettingsAnalytics);
expect(analytics.requests[1]).toHaveProperty('c_type', EventType.SuiteReady);

Expand Down
55 changes: 28 additions & 27 deletions packages/suite-desktop-core/e2e/tests/analytics/toggle.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ test.describe('Analytics Toggle - Enabling and Disabling', { tag: ['@group=other
await expect(settingsPage.analyticsSwitch.locator('input')).not.toBeChecked();

await analyticsPage.continueButton.click(); // Click the button and trigger the request
await expect.poll(() => analytics.requests).toHaveLength(1);
await analytics.waitForAnalyticsRequests();

// assert that only "analytics/dispose" event was fired
const disposeRequest = analytics.requests[0];
const disposeRequest = analytics.findLatestRequestByType(EventType.SettingsAnalytics);
expect(disposeRequest).toHaveProperty('c_type', EventType.SettingsAnalytics);
expect(disposeRequest).toHaveProperty('value', 'false');
expect(disposeRequest).toHaveProperty('c_session_id');
expect(disposeRequest).toHaveProperty('c_instance_id');
expect(disposeRequest).toHaveProperty('c_timestamp');
expect(disposeRequest.c_timestamp).toMatch(/^\d+$/);
expect(disposeRequest?.c_timestamp).toMatch(/^\d+$/);

await page.getByTestId('@onboarding/exit-app-button').click();

Expand All @@ -50,50 +50,49 @@ test.describe('Analytics Toggle - Enabling and Disabling', { tag: ['@group=other
// go to settings, analytics should not enabled and no additional analytics requests should be fired
await settingsPage.navigateTo('application');
await expect(settingsPage.analyticsSwitch.locator('input')).not.toBeChecked();
expect(analytics.requests).toHaveLength(1);

// enable analytics and check "analytics/enable" event was fired
await settingsPage.analyticsSwitch.click();
await expect(settingsPage.analyticsSwitch.locator('input')).toBeChecked();
await expect.poll(() => analytics.requests).toHaveLength(2);
await analytics.waitForAnalyticsRequests();

const enableRequest = analytics.requests[1];
const enableRequest = analytics.findLatestRequestByType(EventType.SettingsAnalytics);
expect(enableRequest).toHaveProperty('c_type', EventType.SettingsAnalytics);
expect(enableRequest).toHaveProperty('c_session_id');
expect(enableRequest).toHaveProperty('c_instance_id');
expect(enableRequest).toHaveProperty('c_timestamp');
expect(enableRequest.c_timestamp).toMatch(/^\d+$/);
expect(analytics.requests).toHaveLength(2);
expect(enableRequest?.c_timestamp).toMatch(/^\d+$/);

// check that timestamps are different
expect(disposeRequest.c_timestamp).not.toEqual(enableRequest.c_timestamp);
expect(disposeRequest?.c_timestamp).not.toEqual(enableRequest?.c_timestamp);

// check that session ids changed after reload
expect(disposeRequest.c_session_id).not.toEqual(enableRequest.c_session_id);
expect(disposeRequest?.c_session_id).not.toEqual(enableRequest?.c_session_id);

// check that instance ids are the same after reload
expect(disposeRequest.c_instance_id).toEqual(enableRequest.c_instance_id);
expect(disposeRequest?.c_instance_id).toEqual(enableRequest?.c_instance_id);

// change fiat and check that it was logged
await page.getByTestId('@settings/fiat-select/input').scrollIntoViewIfNeeded(); // Shouldn't be necessary, but without it the dropdown doesn't open
await page.getByTestId('@settings/fiat-select/input').click();
await page.getByTestId('@settings/fiat-select/option/huf').click();
await expect.poll(() => analytics.requests).toHaveLength(3);
expect(analytics.requests[2]).toHaveProperty('c_type', EventType.SettingsGeneralChangeFiat);
expect(analytics.requests[2]).toHaveProperty('fiat', 'huf');
expect(analytics.requests[2]).toHaveProperty(
'c_instance_id',
analytics.requests[1].c_instance_id,

await analytics.waitForAnalyticsRequests();
const changeFiatRequest = analytics.findLatestRequestByType(
EventType.SettingsGeneralChangeFiat,
);
expect(analytics.requests).toHaveLength(3);
expect(changeFiatRequest).toHaveProperty('c_type', EventType.SettingsGeneralChangeFiat);
expect(changeFiatRequest).toHaveProperty('fiat', 'huf');
expect(changeFiatRequest).toHaveProperty('c_instance_id', enableRequest?.c_instance_id);

// open device modal and check that it was logged
await dashboardPage.openDeviceSwitcher();
await expect.poll(() => analytics.requests).toHaveLength(4);
await analytics.waitForAnalyticsRequests();

const deviceModalRequest = analytics.requests[3];
const deviceModalRequest = analytics.findLatestRequestByType(
EventType.RouterLocationChange,
);
expect(deviceModalRequest).toHaveProperty('c_type', EventType.RouterLocationChange);
expect(analytics.requests).toHaveLength(4);
});

test('should respect enabled analytics in onboarding with following disabling in settings', async ({
Expand All @@ -107,12 +106,12 @@ test.describe('Analytics Toggle - Enabling and Disabling', { tag: ['@group=other
await expect(settingsPage.analyticsSwitch.locator('input')).toBeChecked();

await analyticsPage.continueButton.click(); // Click the button and trigger the request
await expect.poll(() => analytics.requests.length).toBeGreaterThan(2);
await analytics.waitForAnalyticsRequests(2);

// assert that more than 1 event was fired and it was "suite/ready" and "analytics/enable" for sure
expect(analytics.requests.length).toBeGreaterThan(1);
expect(analytics.extractRequestTypes()).toContain(EventType.SuiteReady);
expect(analytics.extractRequestTypes()).toContain(EventType.SettingsAnalytics);
expect(analytics.findLatestRequestByType(EventType.SuiteReady)).toBeDefined();
expect(analytics.findLatestRequestByType(EventType.SettingsAnalytics)).toBeDefined();

// finish onboarding
await page.getByTestId('@onboarding/exit-app-button').click();
Expand All @@ -134,10 +133,12 @@ test.describe('Analytics Toggle - Enabling and Disabling', { tag: ['@group=other
await page.getByTestId('@settings/fiat-select/option/huf').click();

// check that analytics disable event was fired
await expect.poll(() => analytics.requests.length).toBeGreaterThan(3);
expect(analytics.extractRequestTypes()).toContain(EventType.SettingsAnalytics);
await analytics.waitForAnalyticsRequests();
expect(analytics.findLatestRequestByType(EventType.SettingsAnalytics)).toBeDefined();

// check that "settings/general/change-fiat" event was not fired
expect(analytics.extractRequestTypes()).not.toContain(EventType.SettingsGeneralChangeFiat);
expect(
analytics.findLatestRequestByType(EventType.SettingsGeneralChangeFiat),
).not.toBeDefined();
});
});

0 comments on commit cdfe6d2

Please sign in to comment.