From 7d59b6f14a3e75751d8c8444f9ab794a47b2d289 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 15 Mar 2024 14:51:59 -0400 Subject: [PATCH 01/10] test(replay): Fix flakey `extendNetworkBreadcrumbs/fetch` tests See https://github.com/getsentry/sentry-javascript/pull/11140 --- .../fetch/captureRequestHeaders/test.ts | 104 +++++++++--------- .../fetch/captureRequestSize/test.ts | 58 +++++----- .../fetch/captureResponseHeaders/test.ts | 48 ++++---- .../fetch/captureResponseSize/test.ts | 64 ++++++----- 4 files changed, 146 insertions(+), 128 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts index 0b0b37fb1cf6..8f9973f0df20 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts @@ -8,9 +8,7 @@ import { shouldSkipReplayTest, } from '../../../../../utils/replayHelpers'; -// Skipping because this test is flaky -// https://github.com/getsentry/sentry-javascript/issues/11062 -sentryTest.skip('handles empty/missing request headers', async ({ getLocalTestPath, page, browserName }) => { +sentryTest('handles empty/missing request headers', async ({ getLocalTestPath, page, browserName }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } @@ -37,18 +35,20 @@ sentryTest.skip('handles empty/missing request headers', async ({ getLocalTestPa const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/foo', { - method: 'POST', - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/foo', { + method: 'POST', + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -110,25 +110,27 @@ sentryTest('captures request headers as POJO', async ({ getLocalTestPath, page, const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/foo', { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - Cache: 'no-cache', - 'X-Custom-Header': 'foo', - 'X-Test-Header': 'test-value', - }, - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/foo', { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + Cache: 'no-cache', + 'X-Custom-Header': 'foo', + 'X-Test-Header': 'test-value', + }, + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -364,25 +366,27 @@ sentryTest('does not captures request headers if URL does not match', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/bar', { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - Cache: 'no-cache', - 'X-Custom-Header': 'foo', - 'X-Test-Header': 'test-value', - }, - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/bar', { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + Cache: 'no-cache', + 'X-Custom-Header': 'foo', + 'X-Test-Header': 'test-value', + }, + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts index 52857d17479d..d6c9f4813242 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts @@ -8,9 +8,7 @@ import { shouldSkipReplayTest, } from '../../../../../utils/replayHelpers'; -// Skipping because this test is flaky -// https://github.com/getsentry/sentry-javascript/issues/10395 -sentryTest.skip('captures request body size when body is sent', async ({ getLocalTestPath, page }) => { +sentryTest('captures request body size when body is sent', async ({ getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } @@ -37,19 +35,21 @@ sentryTest.skip('captures request body size when body is sent', async ({ getLoca const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/foo', { - method: 'POST', - body: '{"foo":"bar"}', - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/foo', { + method: 'POST', + body: '{"foo":"bar"}', + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -122,21 +122,23 @@ sentryTest('captures request size from non-text request body', async ({ getLocal const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - const blob = new Blob(['Hello world!!'], { type: 'text/html' }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + const blob = new Blob(['Hello world!!'], { type: 'text/html' }); - fetch('http://localhost:7654/foo', { - method: 'POST', - body: blob, - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + fetch('http://localhost:7654/foo', { + method: 'POST', + body: blob, + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts index 8f098627c120..a063bdb90a27 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts @@ -37,14 +37,16 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - fetch('http://localhost:7654/foo').then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + fetch('http://localhost:7654/foo').then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -111,14 +113,16 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - fetch('http://localhost:7654/foo').then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + fetch('http://localhost:7654/foo').then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -191,14 +195,16 @@ sentryTest('does not capture response headers if URL does not match', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - fetch('http://localhost:7654/bar').then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + fetch('http://localhost:7654/bar').then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts index ad3aafe34562..aa2fc9237ab0 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts @@ -43,16 +43,18 @@ sentryTest('captures response size from Content-Length header if available', asy const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/foo').then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/foo').then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -133,16 +135,18 @@ sentryTest('captures response size without Content-Length header', async ({ getL const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/foo').then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/foo').then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -220,18 +224,20 @@ sentryTest('captures response size from non-text response body', async ({ getLoc const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - /* eslint-disable */ - fetch('http://localhost:7654/foo', { - method: 'POST', - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request] = await Promise.all([ + page.evaluate(() => { + /* eslint-disable */ + fetch('http://localhost:7654/foo', { + method: 'POST', + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); From e01233bb69bfa39b8c1589da92457f1039671210 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 15 Mar 2024 14:58:01 -0400 Subject: [PATCH 02/10] formatting --- .../fetch/captureResponseHeaders/test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts index a063bdb90a27..8486cf541a18 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts @@ -45,7 +45,7 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName }); }), requestPromise, - ]); + ]); const eventData = envelopeRequestParser(request); @@ -121,7 +121,7 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { }); }), requestPromise, - ]); + ]); const eventData = envelopeRequestParser(request); @@ -203,7 +203,7 @@ sentryTest('does not capture response headers if URL does not match', async ({ g }); }), requestPromise, - ]); + ]); const eventData = envelopeRequestParser(request); From ac017ba1ea816518f5d16edb49cd5420de61d503 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 15 Mar 2024 15:32:57 -0400 Subject: [PATCH 03/10] collectReplayRequests --- .../fetch/captureResponseHeaders/test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts index 8486cf541a18..8ef33da03fbb 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts @@ -37,7 +37,7 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, {replayRecordingSnapshots}] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/foo').then(() => { // @ts-expect-error Sentry is a global @@ -45,6 +45,7 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName }); }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -63,7 +64,6 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -113,7 +113,7 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, {replayRecordingSnapshots}] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/foo').then(() => { // @ts-expect-error Sentry is a global @@ -121,6 +121,7 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { }); }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -139,7 +140,6 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -195,7 +195,7 @@ sentryTest('does not capture response headers if URL does not match', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, {replayRecordingSnapshots}] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/bar').then(() => { // @ts-expect-error Sentry is a global @@ -203,6 +203,7 @@ sentryTest('does not capture response headers if URL does not match', async ({ g }); }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -221,7 +222,6 @@ sentryTest('does not capture response headers if URL does not match', async ({ g }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { From b4bc852c718d072178f0fae978d956bb1281b823 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 15 Mar 2024 15:56:07 -0400 Subject: [PATCH 04/10] more flakes --- .../fetch/captureRequestHeaders/test.ts | 90 ++++++++++--------- .../fetch/captureRequestSize/test.ts | 8 +- .../fetch/captureResponseHeaders/test.ts | 6 +- .../fetch/captureResponseSize/test.ts | 12 +-- 4 files changed, 60 insertions(+), 56 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts index 8f9973f0df20..19e85e8b03db 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestHeaders/test.ts @@ -35,7 +35,7 @@ sentryTest('handles empty/missing request headers', async ({ getLocalTestPath, p const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo', { @@ -47,6 +47,7 @@ sentryTest('handles empty/missing request headers', async ({ getLocalTestPath, p /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -65,7 +66,6 @@ sentryTest('handles empty/missing request headers', async ({ getLocalTestPath, p }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -110,7 +110,7 @@ sentryTest('captures request headers as POJO', async ({ getLocalTestPath, page, const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo', { @@ -129,6 +129,7 @@ sentryTest('captures request headers as POJO', async ({ getLocalTestPath, page, /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -147,7 +148,6 @@ sentryTest('captures request headers as POJO', async ({ getLocalTestPath, page, }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -197,25 +197,28 @@ sentryTest('captures request headers on Request', async ({ getLocalTestPath, pag const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.evaluate(() => { - const request = new Request('http://localhost:7654/foo', { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - Cache: 'no-cache', - 'X-Custom-Header': 'foo', - }, - }); - /* eslint-disable */ - fetch(request).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + const [, request, { replayRecordingSnapshots }] = await Promise.all([ + page.evaluate(() => { + const request = new Request('http://localhost:7654/foo', { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + Cache: 'no-cache', + 'X-Custom-Header': 'foo', + }, + }); + /* eslint-disable */ + fetch(request).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + replayRequestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -232,7 +235,6 @@ sentryTest('captures request headers on Request', async ({ getLocalTestPath, pag }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -282,25 +284,28 @@ sentryTest('captures request headers as Headers instance', async ({ getLocalTest await page.goto(url); - await page.evaluate(() => { - const headers = new Headers(); - headers.append('Accept', 'application/json'); - headers.append('Content-Type', 'application/json'); - headers.append('Cache', 'no-cache'); - headers.append('X-Custom-Header', 'foo'); + const [, request, { replayRecordingSnapshots }] = await Promise.all([ + page.evaluate(() => { + const headers = new Headers(); + headers.append('Accept', 'application/json'); + headers.append('Content-Type', 'application/json'); + headers.append('Cache', 'no-cache'); + headers.append('X-Custom-Header', 'foo'); - /* eslint-disable */ - fetch('http://localhost:7654/foo', { - method: 'POST', - headers, - }).then(() => { - // @ts-expect-error Sentry is a global - Sentry.captureException('test error'); - }); - /* eslint-enable */ - }); + /* eslint-disable */ + fetch('http://localhost:7654/foo', { + method: 'POST', + headers, + }).then(() => { + // @ts-expect-error Sentry is a global + Sentry.captureException('test error'); + }); + /* eslint-enable */ + }), + requestPromise, + replayRequestPromise, + ]); - const request = await requestPromise; const eventData = envelopeRequestParser(request); expect(eventData.exception?.values).toHaveLength(1); @@ -317,7 +322,6 @@ sentryTest('captures request headers as Headers instance', async ({ getLocalTest }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -366,7 +370,7 @@ sentryTest('does not captures request headers if URL does not match', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/bar', { @@ -385,6 +389,7 @@ sentryTest('does not captures request headers if URL does not match', async ({ g /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -403,7 +408,6 @@ sentryTest('does not captures request headers if URL does not match', async ({ g }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts index d6c9f4813242..dafa896f2cbd 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestSize/test.ts @@ -35,7 +35,7 @@ sentryTest('captures request body size when body is sent', async ({ getLocalTest const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo', { @@ -48,6 +48,7 @@ sentryTest('captures request body size when body is sent', async ({ getLocalTest /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -67,7 +68,6 @@ sentryTest('captures request body size when body is sent', async ({ getLocalTest }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -122,7 +122,7 @@ sentryTest('captures request size from non-text request body', async ({ getLocal const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ const blob = new Blob(['Hello world!!'], { type: 'text/html' }); @@ -137,6 +137,7 @@ sentryTest('captures request size from non-text request body', async ({ getLocal /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -156,7 +157,6 @@ sentryTest('captures request size from non-text request body', async ({ getLocal }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts index 8ef33da03fbb..2d58fd0cd1cf 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts @@ -37,7 +37,7 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, {replayRecordingSnapshots}] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/foo').then(() => { // @ts-expect-error Sentry is a global @@ -113,7 +113,7 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, {replayRecordingSnapshots}] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/foo').then(() => { // @ts-expect-error Sentry is a global @@ -195,7 +195,7 @@ sentryTest('does not capture response headers if URL does not match', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, {replayRecordingSnapshots}] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/bar').then(() => { // @ts-expect-error Sentry is a global diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts index aa2fc9237ab0..312dedfa7eee 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts @@ -43,7 +43,7 @@ sentryTest('captures response size from Content-Length header if available', asy const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo').then(() => { @@ -53,6 +53,7 @@ sentryTest('captures response size from Content-Length header if available', asy /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -72,7 +73,6 @@ sentryTest('captures response size from Content-Length header if available', asy }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -135,7 +135,7 @@ sentryTest('captures response size without Content-Length header', async ({ getL const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo').then(() => { @@ -145,6 +145,7 @@ sentryTest('captures response size without Content-Length header', async ({ getL /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -164,7 +165,6 @@ sentryTest('captures response size without Content-Length header', async ({ getL }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -224,7 +224,7 @@ sentryTest('captures response size from non-text response body', async ({ getLoc const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request] = await Promise.all([ + const [, request, { replayRecordingSnapshots }] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo', { @@ -236,6 +236,7 @@ sentryTest('captures response size from non-text response body', async ({ getLoc /* eslint-enable */ }), requestPromise, + replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -254,7 +255,6 @@ sentryTest('captures response size from non-text response body', async ({ getLoc }, }); - const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { From 9e8e3fdcb6715a6b8ee862e485007109ea70bacc Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 15 Apr 2024 14:56:19 -0400 Subject: [PATCH 05/10] move around replay snapshots --- .../fetch/captureResponseHeaders/test.ts | 12 ++++++------ .../fetch/captureResponseSize/test.ts | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts index 2d58fd0cd1cf..8486cf541a18 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts @@ -37,7 +37,7 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, { replayRecordingSnapshots }] = await Promise.all([ + const [, request] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/foo').then(() => { // @ts-expect-error Sentry is a global @@ -45,7 +45,6 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName }); }), requestPromise, - replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -64,6 +63,7 @@ sentryTest('handles empty headers', async ({ getLocalTestPath, page, browserName }, }); + const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -113,7 +113,7 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, { replayRecordingSnapshots }] = await Promise.all([ + const [, request] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/foo').then(() => { // @ts-expect-error Sentry is a global @@ -121,7 +121,6 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { }); }), requestPromise, - replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -140,6 +139,7 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { }, }); + const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -195,7 +195,7 @@ sentryTest('does not capture response headers if URL does not match', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, { replayRecordingSnapshots }] = await Promise.all([ + const [, request] = await Promise.all([ page.evaluate(() => { fetch('http://localhost:7654/bar').then(() => { // @ts-expect-error Sentry is a global @@ -203,7 +203,6 @@ sentryTest('does not capture response headers if URL does not match', async ({ g }); }), requestPromise, - replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -222,6 +221,7 @@ sentryTest('does not capture response headers if URL does not match', async ({ g }, }); + const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts index 312dedfa7eee..3f1acc745426 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts @@ -43,7 +43,7 @@ sentryTest('captures response size from Content-Length header if available', asy const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, { replayRecordingSnapshots }] = await Promise.all([ + const [, request ] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo').then(() => { @@ -53,7 +53,6 @@ sentryTest('captures response size from Content-Length header if available', asy /* eslint-enable */ }), requestPromise, - replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -73,6 +72,7 @@ sentryTest('captures response size from Content-Length header if available', asy }, }); + const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { @@ -135,7 +135,7 @@ sentryTest('captures response size without Content-Length header', async ({ getL const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request, { replayRecordingSnapshots }] = await Promise.all([ + const [, request] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo').then(() => { @@ -145,7 +145,6 @@ sentryTest('captures response size without Content-Length header', async ({ getL /* eslint-enable */ }), requestPromise, - replayRequestPromise, ]); const eventData = envelopeRequestParser(request); @@ -163,8 +162,9 @@ sentryTest('captures response size without Content-Length header', async ({ getL // NOT set here from body, as this would be async url: 'http://localhost:7654/foo', }, - }); + }) + const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ { data: { From d050be321a3f96af1635208e89fafc3984523d9e Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 15 Apr 2024 15:37:37 -0400 Subject: [PATCH 06/10] upload artifacts on failure --- .github/workflows/flaky-test-detector.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/flaky-test-detector.yml b/.github/workflows/flaky-test-detector.yml index 78496c34b476..383fb878a679 100644 --- a/.github/workflows/flaky-test-detector.yml +++ b/.github/workflows/flaky-test-detector.yml @@ -28,6 +28,8 @@ jobs: name: 'Check tests for flakiness' # Also skip if PR is from master -> develop if: ${{ github.base_ref != 'master' && github.ref != 'refs/heads/master' }} + # Continue on error so that we can upload test results (e.g. playwright traces) on failure + continue-on-error: true steps: - name: Check out current branch uses: actions/checkout@v4 @@ -79,8 +81,18 @@ jobs: browser_integration: dev-packages/browser-integration-tests/suites/**/test.ts - name: Detect flaky tests + id: test run: yarn test:detect-flaky working-directory: dev-packages/browser-integration-tests env: CHANGED_TEST_PATHS: ${{ steps.changed.outputs.browser_integration_files }} TEST_RUN_COUNT: 'AUTO' + + + - name: Artifacts upload + uses: actions/upload-artifact@v4 + if: failure() && steps.test.outcome === 'failure' + with: + name: playwright-test-results + path: test-results + retention-days: 5 From 000e1b7e7ea4e779e0619c689d7bd825735463ec Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 15 Apr 2024 15:46:39 -0400 Subject: [PATCH 07/10] oops dont need continue on error --- .github/workflows/flaky-test-detector.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/flaky-test-detector.yml b/.github/workflows/flaky-test-detector.yml index 383fb878a679..297814d8fb6e 100644 --- a/.github/workflows/flaky-test-detector.yml +++ b/.github/workflows/flaky-test-detector.yml @@ -28,8 +28,6 @@ jobs: name: 'Check tests for flakiness' # Also skip if PR is from master -> develop if: ${{ github.base_ref != 'master' && github.ref != 'refs/heads/master' }} - # Continue on error so that we can upload test results (e.g. playwright traces) on failure - continue-on-error: true steps: - name: Check out current branch uses: actions/checkout@v4 From b50c56335a96e0f69f67c8d928e8e8a26f5d766d Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 15 Apr 2024 15:59:21 -0400 Subject: [PATCH 08/10] format --- .../fetch/captureResponseSize/test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts index 3f1acc745426..63e3611d9d8f 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts @@ -43,7 +43,7 @@ sentryTest('captures response size from Content-Length header if available', asy const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const [, request ] = await Promise.all([ + const [, request] = await Promise.all([ page.evaluate(() => { /* eslint-disable */ fetch('http://localhost:7654/foo').then(() => { @@ -162,7 +162,7 @@ sentryTest('captures response size without Content-Length header', async ({ getL // NOT set here from body, as this would be async url: 'http://localhost:7654/foo', }, - }) + }); const { replayRecordingSnapshots } = await replayRequestPromise; expect(getReplayPerformanceSpans(replayRecordingSnapshots).filter(span => span.op === 'resource.fetch')).toEqual([ From 4534e7e5abba298ddf9753bb1dadea310a3d926c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 15 Apr 2024 15:59:25 -0400 Subject: [PATCH 09/10] js habits --- .github/workflows/flaky-test-detector.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/flaky-test-detector.yml b/.github/workflows/flaky-test-detector.yml index 297814d8fb6e..57a73f5470a9 100644 --- a/.github/workflows/flaky-test-detector.yml +++ b/.github/workflows/flaky-test-detector.yml @@ -89,7 +89,7 @@ jobs: - name: Artifacts upload uses: actions/upload-artifact@v4 - if: failure() && steps.test.outcome === 'failure' + if: failure() && steps.test.outcome == 'failure' with: name: playwright-test-results path: test-results From cfdcb3a0b159a480b8d13394db1a9e2b458ca2bc Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 13 May 2024 11:18:03 -0400 Subject: [PATCH 10/10] Update .github/workflows/flaky-test-detector.yml Co-authored-by: Francesco Novy --- .github/workflows/flaky-test-detector.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/flaky-test-detector.yml b/.github/workflows/flaky-test-detector.yml index 8ea4806d14b9..44edf51fd45d 100644 --- a/.github/workflows/flaky-test-detector.yml +++ b/.github/workflows/flaky-test-detector.yml @@ -86,7 +86,6 @@ jobs: CHANGED_TEST_PATHS: ${{ steps.changed.outputs.browser_integration_files }} TEST_RUN_COUNT: 'AUTO' - - name: Artifacts upload uses: actions/upload-artifact@v4 if: failure() && steps.test.outcome == 'failure'