Skip to content

Commit

Permalink
test: Only run specified integration tests with firefox
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mydea committed Jul 21, 2023
1 parent df7bb25 commit ed83d63
Show file tree
Hide file tree
Showing 23 changed files with 144 additions and 141 deletions.
6 changes: 3 additions & 3 deletions packages/browser-integration-tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ Tests can be run locally using the latest version of Chromium with:

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

`yarn test --browser='firefox'`
`yarn test --browser='webkit'`
`yarn test --project='firefox'`
`yarn test --project='webkit'`

Or to run on all three browsers:

`yarn test --browser='all'`
`yarn test:all`

To filter tests by their title:

Expand Down
7 changes: 4 additions & 3 deletions packages/browser-integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
"fix:prettier": "prettier --write \"{suites,utils}/**/*.ts\"",
"type-check": "tsc",
"pretest": "yarn clean && yarn type-check",
"test": "playwright test ./suites",
"test": "playwright test ./suites --project='chromium'",
"test:all": "playwright test ./suites",
"test:bundle:es5": "PW_BUNDLE=bundle_es5 yarn test",
"test:bundle:es5:min": "PW_BUNDLE=bundle_es5_min yarn test",
"test:bundle:es6": "PW_BUNDLE=bundle_es6 yarn test",
Expand All @@ -40,8 +41,8 @@
"test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
"test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
"test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
"test:ci": "playwright test ./suites --browser='all' --reporter='line'",
"test:update-snapshots": "yarn test --update-snapshots --browser='all' && yarn test --update-snapshots",
"test:ci": "playwright test ./suites --reporter='line'",
"test:update-snapshots": "yarn test:all --update-snapshots",
"test:detect-flaky": "ts-node scripts/detectFlakyTests.ts",
"validate:es5": "es-check es5 'fixtures/loader.js'"
},
Expand Down
17 changes: 17 additions & 0 deletions packages/browser-integration-tests/playwright.config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { PlaywrightTestConfig } from '@playwright/test';
import { devices } from '@playwright/test';

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

projects: [
{
name: 'chromium',
use: devices['Desktop Chrome'],
},
{
name: 'webkit',
use: devices['Desktop Safari'],
},
{
name: 'firefox',
grep: /@firefox/i,
use: devices['Desktop Firefox'],
},
],

globalSetup: require.resolve('./playwright.setup.ts'),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ ${changedPaths.join('\n')}
const cp = childProcess.spawn(
`yarn playwright test ${
testPaths.length ? testPaths.join(' ') : './suites'
} --browser='all' --reporter='line' --repeat-each ${runCount}`,
} --reporter='line' --repeat-each ${runCount}`,
{ shell: true, cwd },
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import type { Event } from '@sentry/types';
import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';

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

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import {
sentryTest(
'[buffer-mode] manually start buffer mode and capture buffer',
async ({ getLocalTestPath, page, browserName }) => {
// This was sometimes flaky on firefox/webkit, so skipping for now
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
// This was sometimes flaky on webkit, so skipping for now
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ sentryTest(
sentryTest(
'replay recording should contain a click breadcrumb when a button is clicked',
async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => {
// TODO(replay): This is flakey on firefox and webkit where clicks are flakey
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
// TODO(replay): This is flakey on webkit where clicks are flakey
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import {
sentryTest(
'[error-mode] should start recording and switch to session mode once an error is thrown',
async ({ getLocalTestPath, page, browserName }) => {
// This was sometimes flaky on firefox/webkit, so skipping for now
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
// This was sometimes flaky on webkit, so skipping for now
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {
sentryTest(
'[session-mode] replay event should contain an error id of an error that occurred during session recording',
async ({ getLocalTestPath, page, browserName, forceFlushReplay }) => {
// Skipping this in firefox/webkit because it is flakey there
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
// Skipping this in webkit because it is flakey there
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest }
sentryTest(
'handles large mutations with default options',
async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => {
if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) {
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
sentryTest(
'handles large mutations by stopping replay when `mutationLimit` configured',
async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => {
if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) {
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ function isInputMutation(
sentryTest(
'should mask input initial value and its changes',
async ({ browserName, forceFlushReplay, getLocalTestPath, page }) => {
// TODO(replay): This is flakey on firefox and webkit (~1%) where we do not always get the latest mutation.
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
// TODO(replay): This is flakey on webkit (~1%) where we do not always get the latest mutation.
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ function isInputMutation(
sentryTest(
'should mask input initial value and its changes from `maskAllInputs` and allow unmasked selector',
async ({ browserName, forceFlushReplay, getLocalTestPath, page }) => {
// TODO(replay): This is flakey on firefox and webkit (~1%) where we do not always get the latest mutation.
if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) {
// TODO(replay): This is flakey on webkit (~1%) where we do not always get the latest mutation.
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import { expectedFetchPerformanceSpan, expectedXHRPerformanceSpan } from '../../
import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';

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

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import {
// Session should expire after 2s - keep in sync with init.js
const SESSION_TIMEOUT = 2000;

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ sentryTest('captures multi click when not detecting slow click', async ({ getLoc
});

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,61 +126,57 @@ sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => {
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
});

sentryTest(
'immediate mutation does not trigger slow click',
async ({ forceFlushReplay, browserName, getLocalTestUrl, page }) => {
// This test seems to only be flakey on firefox
if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);

await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
sentryTest('immediate mutation does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);

await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestUrl({ testDir: __dirname });
const url = await getLocalTestUrl({ testDir: __dirname });

await page.goto(url);
await forceFlushReplay();
await reqPromise0;
await page.goto(url);
await forceFlushReplay();
await reqPromise0;

const reqPromise1 = waitForReplayRequest(page, (event, res) => {
const { breadcrumbs } = getCustomRecordingEvents(res);
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
const { breadcrumbs } = getCustomRecordingEvents(res);

return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
});
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
});

await page.click('#mutationButtonImmediately');

await page.click('#mutationButtonImmediately');

const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);

expect(breadcrumbs).toEqual([
{
category: 'ui.click',
data: {
node: {
attributes: {
id: 'mutationButtonImmediately',
},
id: expect.any(Number),
tagName: 'button',
textContent: '******* ******** ***********',
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);

expect(breadcrumbs).toEqual([
{
category: 'ui.click',
data: {
node: {
attributes: {
id: 'mutationButtonImmediately',
},
nodeId: expect.any(Number),
id: expect.any(Number),
tagName: 'button',
textContent: '******* ******** ***********',
},
message: 'body > button#mutationButtonImmediately',
timestamp: expect.any(Number),
type: 'default',
nodeId: expect.any(Number),
},
]);
},
);
message: 'body > button#mutationButtonImmediately',
timestamp: expect.any(Number),
type: 'default',
},
]);
});

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

sentryTest('mouseDown events are considered', async ({ browserName, getLocalTestUrl, page }) => {
// This test seems to only be flakey on firefox
if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
sentryTest('mouseDown events are considered', async ({ getLocalTestUrl, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}

Expand Down
Loading

0 comments on commit ed83d63

Please sign in to comment.