Skip to content

Commit

Permalink
fix(tests): Update loader integration tests to avoid flakes. (#8601)
Browse files Browse the repository at this point in the history
Playwright's event listeners and `page.goto` functions can occasionally
end up in race condition, even when they are invoked in the correct
order.

The workaround is to invoke them in `Promise.all`, unless there's a
specific need to separate them.
  • Loading branch information
onurtemizkan committed Jul 21, 2023
1 parent 67c3b6a commit da07542
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 61 deletions.
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('captureException works', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

expect(eventData.message).toBe('Test exception');
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('error handler works with a recursive custom error handler', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);
expect(eventData.exception?.values?.length).toBe(1);
expect(eventData.exception?.values?.[0]?.value).toBe('window.doSomethingWrong is not a function');
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser,waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('error handler works', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);
expect(eventData.exception?.values?.length).toBe(1);
expect(eventData.exception?.values?.[0]?.value).toBe('window.doSomethingWrong is not a function');
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser,waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('error handler works for later errors', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

expect(eventData.exception?.values?.length).toBe(1);
expect(eventData.exception?.values?.[0]?.value).toBe('window.doSomethingWrong is not a function');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser,shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
import {
envelopeRequestParser,
shouldSkipTracingTest,
waitForTransactionRequestOnUrl,
} from '../../../../utils/helpers';

sentryTest('should create a pageload transaction', async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const req = waitForTransactionRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForTransactionRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);
const timeOrigin = await page.evaluate<number>('window._testBaseTimestamp');

const { start_timestamp: startTimestamp } = eventData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from 'path';

import { sentryTest, TEST_HOST } from '../../../../utils/fixtures';
import { LOADER_CONFIGS } from '../../../../utils/generatePlugin';
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

const bundle = process.env.PW_BUNDLE || '';
const isLazy = LOADER_CONFIGS[bundle]?.lazy;
Expand Down Expand Up @@ -40,13 +40,10 @@ sentryTest('it does not download the SDK if the SDK was loaded in the meanwhile'
return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue();
});

const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname, skipRouteHandler: true });
const req = await waitForErrorRequestOnUrl(page, url);

await page.goto(url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

await waitForFunction(() => cdnLoadedCount === 2);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('captureException works', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

expect(eventData.message).toBe('Test exception');
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('captureException works inside of onLoad', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

expect(eventData.message).toBe('Test exception');
});
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
import {
envelopeRequestParser,
shouldSkipTracingTest,
waitForTransactionRequestOnUrl,
} from '../../../../utils/helpers';

sentryTest('should handle custom added BrowserTracing integration', async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const req = waitForTransactionRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForTransactionRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);
const timeOrigin = await page.evaluate<number>('window._testBaseTimestamp');

const { start_timestamp: startTimestamp } = eventData;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('error handler works', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

expect(eventData.exception?.values?.length).toBe(1);
expect(eventData.exception?.values?.[0]?.value).toBe('window.doSomethingWrong is not a function');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser,waitForErrorRequest } from '../../../../utils/helpers';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('error handler works for later errors', async ({ getLocalTestUrl, page }) => {
const req = waitForErrorRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForErrorRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);

expect(eventData.exception?.values?.length).toBe(1);
expect(eventData.exception?.values?.[0]?.value).toBe('window.doSomethingWrong is not a function');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser,shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
import {
envelopeRequestParser,
shouldSkipTracingTest,
waitForTransactionRequestOnUrl,
} from '../../../../utils/helpers';

sentryTest('should create a pageload transaction', async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const req = waitForTransactionRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);
const req = await waitForTransactionRequestOnUrl(page, url);

const eventData = envelopeRequestParser(await req);
const eventData = envelopeRequestParser(req);
const timeOrigin = await page.evaluate<number>('window._testBaseTimestamp');

const { start_timestamp: startTimestamp } = eventData;
Expand Down
10 changes: 10 additions & 0 deletions packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ async function getSentryEvents(page: Page, url?: string): Promise<Array<Event>>
return eventsHandle.jsonValue();
}

export async function waitForErrorRequestOnUrl(page: Page, url: string): Promise<Request> {
const [req] = await Promise.all([waitForErrorRequest(page), page.goto(url)]);
return req;
}

export async function waitForTransactionRequestOnUrl(page: Page, url: string): Promise<Request> {
const [req] = await Promise.all([waitForTransactionRequest(page), page.goto(url)]);
return req;
}

export function waitForErrorRequest(page: Page): Promise<Request> {
return page.waitForRequest(req => {
const postData = req.postData();
Expand Down

0 comments on commit da07542

Please sign in to comment.