Skip to content

Commit

Permalink
test: Only run specified integration tests with firefox (#8603)
Browse files Browse the repository at this point in the history
As a possible fix for
#8589, this sets up
our integration tests to only run tests in firefox when the test
contains `@firefox`. This way, we manually tag tests we want to run in
firefox, as the tests are quite often flaky.

We may think about reverting this once
microsoft/playwright#12182 is fixed.
  • Loading branch information
mydea committed Jul 25, 2023
1 parent 0cdd472 commit 428598d
Show file tree
Hide file tree
Showing 38 changed files with 151 additions and 1,504 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
9 changes: 5 additions & 4 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 @@ -33,15 +34,15 @@
"test:bundle:tracing:replay:es6:min": "PW_BUNDLE=bundle_tracing_replay_es6_min yarn test",
"test:cjs": "PW_BUNDLE=cjs yarn test",
"test:esm": "PW_BUNDLE=esm yarn test",
"test:loader": "playwright test ./loader-suites",
"test:loader": "playwright test ./loader-suites --project='chromium'",
"test:loader:base": "PW_BUNDLE=loader_base yarn test:loader",
"test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
"test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
"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

This file was deleted.

Loading

0 comments on commit 428598d

Please sign in to comment.