Skip to content

Commit fec22b8

Browse files
committed
test: Only run specified integration tests with firefox
This changes it so you have to mark integration tests you want to run in firefox with `@firefox`. We've been skipping a few of them and had often flakiness issues, probably due to microsoft/playwright#12182.
1 parent f669f66 commit fec22b8

File tree

23 files changed

+144
-141
lines changed

23 files changed

+144
-141
lines changed

packages/browser-integration-tests/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ Tests can be run locally using the latest version of Chromium with:
4747

4848
To run tests with a different browser such as `firefox` or `webkit`:
4949

50-
`yarn test --browser='firefox'`
51-
`yarn test --browser='webkit'`
50+
`yarn test --project='firefox'`
51+
`yarn test --project='webkit'`
5252

5353
Or to run on all three browsers:
5454

55-
`yarn test --browser='all'`
55+
`yarn test:all`
5656

5757
To filter tests by their title:
5858

packages/browser-integration-tests/package.json

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
"fix:prettier": "prettier --write \"{suites,utils}/**/*.ts\"",
1919
"type-check": "tsc",
2020
"pretest": "yarn clean && yarn type-check",
21-
"test": "playwright test ./suites",
21+
"test": "playwright test ./suites --project='chromium'",
22+
"test:all": "playwright test ./suites",
2223
"test:bundle:es5": "PW_BUNDLE=bundle_es5 yarn test",
2324
"test:bundle:es5:min": "PW_BUNDLE=bundle_es5_min yarn test",
2425
"test:bundle:es6": "PW_BUNDLE=bundle_es6 yarn test",
@@ -40,8 +41,8 @@
4041
"test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
4142
"test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
4243
"test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
43-
"test:ci": "playwright test ./suites --browser='all' --reporter='line'",
44-
"test:update-snapshots": "yarn test --update-snapshots --browser='all' && yarn test --update-snapshots",
44+
"test:ci": "playwright test ./suites --reporter='line'",
45+
"test:update-snapshots": "yarn test --update-snapshots && yarn test --update-snapshots",
4546
"test:detect-flaky": "ts-node scripts/detectFlakyTests.ts",
4647
"validate:es5": "es-check es5 'fixtures/loader.js'"
4748
},

packages/browser-integration-tests/playwright.config.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { PlaywrightTestConfig } from '@playwright/test';
2+
import { devices } from '@playwright/test';
23

34
const config: PlaywrightTestConfig = {
45
retries: 0,
@@ -8,6 +9,22 @@ const config: PlaywrightTestConfig = {
89
// Note that 3 is a random number selected to work well with our CI setup
910
workers: process.env.CI ? 3 : undefined,
1011

12+
projects: [
13+
{
14+
name: 'chromium',
15+
use: devices['Desktop Chrome'],
16+
},
17+
{
18+
name: 'webkit',
19+
use: devices['Desktop Safari'],
20+
},
21+
{
22+
name: 'firefox',
23+
grep: /@firefox/i,
24+
use: devices['Desktop Firefox'],
25+
},
26+
],
27+
1128
globalSetup: require.resolve('./playwright.setup.ts'),
1229
};
1330

packages/browser-integration-tests/scripts/detectFlakyTests.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ ${changedPaths.join('\n')}
4646
const cp = childProcess.spawn(
4747
`yarn playwright test ${
4848
testPaths.length ? testPaths.join(' ') : './suites'
49-
} --browser='all' --reporter='line' --repeat-each ${runCount}`,
49+
} --reporter='line' --repeat-each ${runCount}`,
5050
{ shell: true, cwd },
5151
);
5252

packages/browser-integration-tests/suites/public-api/captureException/errorEvent/test.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@ import type { Event } from '@sentry/types';
44
import { sentryTest } from '../../../../utils/fixtures';
55
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
66

7-
sentryTest('should capture an ErrorEvent', async ({ getLocalTestPath, page, browserName }) => {
8-
// On Firefox, the ErrorEvent has the `error` property and thus is handled separately
9-
if (browserName === 'firefox') {
10-
sentryTest.skip();
11-
}
7+
sentryTest('should capture an ErrorEvent', async ({ getLocalTestPath, page }) => {
128
const url = await getLocalTestPath({ testDir: __dirname });
139

1410
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);

packages/browser-integration-tests/suites/replay/bufferMode/test.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import {
1616
sentryTest(
1717
'[buffer-mode] manually start buffer mode and capture buffer',
1818
async ({ getLocalTestPath, page, browserName }) => {
19-
// This was sometimes flaky on firefox/webkit, so skipping for now
20-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
19+
// This was sometimes flaky on webkit, so skipping for now
20+
if (shouldSkipReplayTest() || browserName === 'webkit') {
2121
sentryTest.skip();
2222
}
2323

@@ -159,8 +159,8 @@ sentryTest(
159159
sentryTest(
160160
'[buffer-mode] manually start buffer mode and capture buffer, but do not continue as session',
161161
async ({ getLocalTestPath, page, browserName }) => {
162-
// This was sometimes flaky on firefox/webkit, so skipping for now
163-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
162+
// This was sometimes flaky on webkit, so skipping for now
163+
if (shouldSkipReplayTest() || browserName === 'webkit') {
164164
sentryTest.skip();
165165
}
166166

@@ -287,8 +287,7 @@ sentryTest(
287287
sentryTest(
288288
'[buffer-mode] can sample on each error event',
289289
async ({ getLocalTestPath, page, browserName, enableConsole }) => {
290-
// This was sometimes flaky on firefox/webkit, so skipping for now
291-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
290+
if (shouldSkipReplayTest() || browserName === 'webkit') {
292291
sentryTest.skip();
293292
}
294293

packages/browser-integration-tests/suites/replay/customEvents/test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ sentryTest(
8181
sentryTest(
8282
'replay recording should contain a click breadcrumb when a button is clicked',
8383
async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => {
84-
// TODO(replay): This is flakey on firefox and webkit where clicks are flakey
85-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
84+
// TODO(replay): This is flakey on webkit where clicks are flakey
85+
if (shouldSkipReplayTest() || browserName === 'webkit') {
8686
sentryTest.skip();
8787
}
8888

@@ -177,8 +177,8 @@ sentryTest(
177177
sentryTest(
178178
'replay recording should contain an "options" breadcrumb for Replay SDK configuration',
179179
async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => {
180-
// TODO(replay): This is flakey on firefox and webkit where clicks are flakey
181-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
180+
// TODO(replay): This is flakey on webkit where clicks are flakey
181+
if (shouldSkipReplayTest() || browserName === 'webkit') {
182182
sentryTest.skip();
183183
}
184184

packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import {
1818
sentryTest(
1919
'[error-mode] should start recording and switch to session mode once an error is thrown',
2020
async ({ getLocalTestPath, page, browserName }) => {
21-
// This was sometimes flaky on firefox/webkit, so skipping for now
22-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
21+
// This was sometimes flaky on webkit, so skipping for now
22+
if (shouldSkipReplayTest() || browserName === 'webkit') {
2323
sentryTest.skip();
2424
}
2525

packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import {
1313
sentryTest(
1414
'[session-mode] replay event should contain an error id of an error that occurred during session recording',
1515
async ({ getLocalTestPath, page, browserName, forceFlushReplay }) => {
16-
// Skipping this in firefox/webkit because it is flakey there
17-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
16+
// Skipping this in webkit because it is flakey there
17+
if (shouldSkipReplayTest() || browserName === 'webkit') {
1818
sentryTest.skip();
1919
}
2020

@@ -86,8 +86,8 @@ sentryTest(
8686
sentryTest(
8787
'[session-mode] replay event should not contain an error id of a dropped error while recording',
8888
async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => {
89-
// Skipping this in firefox/webkit because it is flakey there
90-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
89+
// Skipping this in webkit because it is flakey there
90+
if (shouldSkipReplayTest() || browserName === 'webkit') {
9191
sentryTest.skip();
9292
}
9393

packages/browser-integration-tests/suites/replay/largeMutations/defaultOptions/test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest }
66
sentryTest(
77
'handles large mutations with default options',
88
async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => {
9-
if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) {
9+
if (shouldSkipReplayTest() || browserName === 'webkit') {
1010
sentryTest.skip();
1111
}
1212

packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
sentryTest(
1212
'handles large mutations by stopping replay when `mutationLimit` configured',
1313
async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => {
14-
if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) {
14+
if (shouldSkipReplayTest() || browserName === 'webkit') {
1515
sentryTest.skip();
1616
}
1717

packages/browser-integration-tests/suites/replay/privacyInput/test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ function isInputMutation(
1919
sentryTest(
2020
'should mask input initial value and its changes',
2121
async ({ browserName, forceFlushReplay, getLocalTestPath, page }) => {
22-
// TODO(replay): This is flakey on firefox and webkit (~1%) where we do not always get the latest mutation.
23-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
22+
// TODO(replay): This is flakey on webkit (~1%) where we do not always get the latest mutation.
23+
if (shouldSkipReplayTest() || browserName === 'webkit') {
2424
sentryTest.skip();
2525
}
2626

@@ -91,8 +91,8 @@ sentryTest(
9191
sentryTest(
9292
'should mask textarea initial value and its changes',
9393
async ({ browserName, forceFlushReplay, getLocalTestPath, page }) => {
94-
// TODO(replay): This is flakey on firefox and webkit (~1%) where we do not always get the latest mutation.
95-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
94+
// TODO(replay): This is flakey on webkit (~1%) where we do not always get the latest mutation.
95+
if (shouldSkipReplayTest() || browserName === 'webkit') {
9696
sentryTest.skip();
9797
}
9898

packages/browser-integration-tests/suites/replay/privacyInputMaskAll/test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ function isInputMutation(
1919
sentryTest(
2020
'should mask input initial value and its changes from `maskAllInputs` and allow unmasked selector',
2121
async ({ browserName, forceFlushReplay, getLocalTestPath, page }) => {
22-
// TODO(replay): This is flakey on firefox and webkit (~1%) where we do not always get the latest mutation.
23-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
22+
// TODO(replay): This is flakey on webkit (~1%) where we do not always get the latest mutation.
23+
if (shouldSkipReplayTest() || browserName === 'webkit') {
2424
sentryTest.skip();
2525
}
2626

@@ -78,8 +78,8 @@ sentryTest(
7878
sentryTest(
7979
'should mask textarea initial value and its changes from `maskAllInputs` and allow unmasked selector',
8080
async ({ browserName, forceFlushReplay, getLocalTestPath, page }) => {
81-
// TODO(replay): This is flakey on firefox and webkit (~1%) where we do not always get the latest mutation.
82-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
81+
// TODO(replay): This is flakey on webkit (~1%) where we do not always get the latest mutation.
82+
if (shouldSkipReplayTest() || browserName === 'webkit') {
8383
sentryTest.skip();
8484
}
8585

packages/browser-integration-tests/suites/replay/requests/test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ import { expectedFetchPerformanceSpan, expectedXHRPerformanceSpan } from '../../
55
import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';
66

77
sentryTest('replay recording should contain fetch request span', async ({ getLocalTestPath, page, browserName }) => {
8-
// For some reason, observing and waiting for requests in firefox/webkit is extremely flaky.
9-
// We therefore skip this test for firefox and only test on chromium.
108
// Possibly related: https://github.com/microsoft/playwright/issues/11390
11-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
9+
if (shouldSkipReplayTest() || browserName === 'webkit') {
1210
sentryTest.skip();
1311
}
1412

@@ -48,9 +46,7 @@ sentryTest('replay recording should contain fetch request span', async ({ getLoc
4846
});
4947

5048
sentryTest('replay recording should contain XHR request span', async ({ getLocalTestPath, page, browserName }) => {
51-
// For some reason, observing and waiting for requests in firefox/webkit is extremely flaky.
52-
// We therefore skip this test for firefox and only test on chromium.
53-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
49+
if (shouldSkipReplayTest() || browserName === 'webkit') {
5450
sentryTest.skip();
5551
}
5652

packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ import {
1414
// Session should expire after 2s - keep in sync with init.js
1515
const SESSION_TIMEOUT = 2000;
1616

17-
sentryTest('handles an expired session', async ({ browserName, getLocalTestPath, page }) => {
18-
// This test seems to only be flakey on firefox
19-
if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
17+
sentryTest('handles an expired session', async ({ getLocalTestPath, page }) => {
18+
if (shouldSkipReplayTest()) {
2019
sentryTest.skip();
2120
}
2221

packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,7 @@ sentryTest('captures multi click when not detecting slow click', async ({ getLoc
102102
});
103103

104104
sentryTest('captures multiple multi clicks', async ({ getLocalTestUrl, page, forceFlushReplay, browserName }) => {
105-
// This test seems to only be flakey on firefox and webkit
106-
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
105+
if (shouldSkipReplayTest() || browserName === 'webkit') {
107106
sentryTest.skip();
108107
}
109108

packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts

Lines changed: 44 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -126,61 +126,57 @@ sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => {
126126
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
127127
});
128128

129-
sentryTest(
130-
'immediate mutation does not trigger slow click',
131-
async ({ forceFlushReplay, browserName, getLocalTestUrl, page }) => {
132-
// This test seems to only be flakey on firefox
133-
if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
134-
sentryTest.skip();
135-
}
136-
137-
const reqPromise0 = waitForReplayRequest(page, 0);
138-
139-
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
140-
return route.fulfill({
141-
status: 200,
142-
contentType: 'application/json',
143-
body: JSON.stringify({ id: 'test-id' }),
144-
});
129+
sentryTest('immediate mutation does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => {
130+
if (shouldSkipReplayTest()) {
131+
sentryTest.skip();
132+
}
133+
134+
const reqPromise0 = waitForReplayRequest(page, 0);
135+
136+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
137+
return route.fulfill({
138+
status: 200,
139+
contentType: 'application/json',
140+
body: JSON.stringify({ id: 'test-id' }),
145141
});
142+
});
146143

147-
const url = await getLocalTestUrl({ testDir: __dirname });
144+
const url = await getLocalTestUrl({ testDir: __dirname });
148145

149-
await page.goto(url);
150-
await forceFlushReplay();
151-
await reqPromise0;
146+
await page.goto(url);
147+
await forceFlushReplay();
148+
await reqPromise0;
152149

153-
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
154-
const { breadcrumbs } = getCustomRecordingEvents(res);
150+
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
151+
const { breadcrumbs } = getCustomRecordingEvents(res);
155152

156-
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
157-
});
153+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
154+
});
155+
156+
await page.click('#mutationButtonImmediately');
158157

159-
await page.click('#mutationButtonImmediately');
160-
161-
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
162-
163-
expect(breadcrumbs).toEqual([
164-
{
165-
category: 'ui.click',
166-
data: {
167-
node: {
168-
attributes: {
169-
id: 'mutationButtonImmediately',
170-
},
171-
id: expect.any(Number),
172-
tagName: 'button',
173-
textContent: '******* ******** ***********',
158+
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
159+
160+
expect(breadcrumbs).toEqual([
161+
{
162+
category: 'ui.click',
163+
data: {
164+
node: {
165+
attributes: {
166+
id: 'mutationButtonImmediately',
174167
},
175-
nodeId: expect.any(Number),
168+
id: expect.any(Number),
169+
tagName: 'button',
170+
textContent: '******* ******** ***********',
176171
},
177-
message: 'body > button#mutationButtonImmediately',
178-
timestamp: expect.any(Number),
179-
type: 'default',
172+
nodeId: expect.any(Number),
180173
},
181-
]);
182-
},
183-
);
174+
message: 'body > button#mutationButtonImmediately',
175+
timestamp: expect.any(Number),
176+
type: 'default',
177+
},
178+
]);
179+
});
184180

185181
sentryTest('inline click handler does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => {
186182
if (shouldSkipReplayTest()) {
@@ -234,9 +230,8 @@ sentryTest('inline click handler does not trigger slow click', async ({ forceFlu
234230
]);
235231
});
236232

237-
sentryTest('mouseDown events are considered', async ({ browserName, getLocalTestUrl, page }) => {
238-
// This test seems to only be flakey on firefox
239-
if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
233+
sentryTest('mouseDown events are considered', async ({ getLocalTestUrl, page }) => {
234+
if (shouldSkipReplayTest()) {
240235
sentryTest.skip();
241236
}
242237

0 commit comments

Comments
 (0)