fix(tests): Update loader integration tests to avoid flakes. (#8601)

onurtemizkan · web-flow · commit da0754205363 · 2023-07-21T08:56:49.000+02:00
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.
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/captureException/test.ts b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/captureException/test.ts
@@ -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');
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/customOnErrorHandler/test.ts b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/customOnErrorHandler/test.ts
@@ -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');
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/errorHandler/test.ts b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/errorHandler/test.ts
@@ -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');
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/errorHandlerLater/test.ts b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/errorHandlerLater/test.ts
@@ -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');
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/pageloadTransaction/test.ts b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/pageloadTransaction/test.ts
@@ -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;
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/sdkLoadedInMeanwhile/test.ts b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/sdkLoadedInMeanwhile/test.ts
@@ -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;
@@ -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);
 
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/captureException/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/captureException/test.ts
@@ -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');
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/captureExceptionInOnLoad/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/captureExceptionInOnLoad/test.ts
@@ -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');
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customBrowserTracing/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/customBrowserTracing/test.ts
@@ -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;
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/errorHandler/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/errorHandler/test.ts
@@ -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');
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/errorHandlerLater/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/errorHandlerLater/test.ts
@@ -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');
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/pageloadTransaction/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/pageloadTransaction/test.ts
@@ -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;
diff --git a/packages/browser-integration-tests/utils/helpers.ts b/packages/browser-integration-tests/utils/helpers.ts
@@ -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();
