Skip to content

Commit 12d5acf

Browse files
authored
test(feedback): Re-add feedback CDN tests (#11890)
This reverts #11888, and ensures the feedback tests actually work on CDN. For this, we now ensure to serve this locally, so this will work also on release branches. It means you have to use `getLocalTestUrl` instead of `getLocalTestPath` to work. (side note: We can/should probably just remove `getLocalTestPath` overall 🤔 URL based is much more realistic and IMHO better. I'll do that in a follow up, maybe.
1 parent d5a3ded commit 12d5acf

File tree

9 files changed

+59
-19
lines changed

9 files changed

+59
-19
lines changed

dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { expect } from '@playwright/test';
22

3-
import { sentryTest } from '../../../utils/fixtures';
3+
import { TEST_HOST, sentryTest } from '../../../utils/fixtures';
44
import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../utils/helpers';
55

6-
sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
6+
sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => {
77
if (shouldSkipFeedbackTest()) {
88
sentryTest.skip();
99
}
@@ -31,7 +31,7 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
3131
});
3232
});
3333

34-
const url = await getLocalTestPath({ testDir: __dirname });
34+
const url = await getLocalTestUrl({ testDir: __dirname });
3535

3636
await page.goto(url);
3737
await page.getByText('Report a Bug').click();
@@ -51,7 +51,7 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
5151
message: 'my example feedback',
5252
name: 'Jane Doe',
5353
source: 'widget',
54-
url: expect.stringContaining('/dist/index.html'),
54+
url: `${TEST_HOST}/index.html`,
5555
},
5656
trace: {
5757
trace_id: expect.stringMatching(/\w{32}/),
@@ -69,7 +69,7 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
6969
packages: expect.anything(),
7070
},
7171
request: {
72-
url: expect.stringContaining('/dist/index.html'),
72+
url: `${TEST_HOST}/index.html`,
7373
headers: {
7474
'User-Agent': expect.stringContaining(''),
7575
},

dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect } from '@playwright/test';
22

3-
import { sentryTest } from '../../../../utils/fixtures';
3+
import { TEST_HOST, sentryTest } from '../../../../utils/fixtures';
44
import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../../utils/helpers';
55
import {
66
collectReplayRequests,
@@ -9,7 +9,7 @@ import {
99
waitForReplayRequest,
1010
} from '../../../../utils/replayHelpers';
1111

12-
sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPath, page }) => {
12+
sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestUrl, page }) => {
1313
if (shouldSkipFeedbackTest() || shouldSkipReplayTest()) {
1414
sentryTest.skip();
1515
}
@@ -39,7 +39,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat
3939
});
4040
});
4141

42-
const url = await getLocalTestPath({ testDir: __dirname });
42+
const url = await getLocalTestUrl({ testDir: __dirname });
4343

4444
await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);
4545

@@ -87,7 +87,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat
8787
name: 'Jane Doe',
8888
replay_id: replayEvent.event_id,
8989
source: 'widget',
90-
url: expect.stringContaining('/dist/index.html'),
90+
url: `${TEST_HOST}/index.html`,
9191
},
9292
trace: {
9393
trace_id: expect.stringMatching(/\w{32}/),
@@ -105,7 +105,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat
105105
packages: expect.anything(),
106106
},
107107
request: {
108-
url: expect.stringContaining('/dist/index.html'),
108+
url: `${TEST_HOST}/index.html`,
109109
headers: {
110110
'User-Agent': expect.stringContaining(''),
111111
},

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ sentryTest('window.open() is considered for slow click', async ({ getLocalTestUr
1616
});
1717
});
1818

19+
await page.route('http://example.com/', route => {
20+
return route.fulfill({
21+
status: 200,
22+
contentType: 'application/json',
23+
body: JSON.stringify({}),
24+
});
25+
});
26+
1927
const url = await getLocalTestUrl({ testDir: __dirname });
2028

2129
await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]);

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,12 +311,12 @@ sentryTest(
311311

312312
sentryTest(
313313
'user feedback event after navigation has navigation traceId in headers',
314-
async ({ getLocalTestPath, page }) => {
314+
async ({ getLocalTestUrl, page }) => {
315315
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
316316
sentryTest.skip();
317317
}
318318

319-
const url = await getLocalTestPath({ testDir: __dirname });
319+
const url = await getLocalTestUrl({ testDir: __dirname });
320320

321321
// ensure pageload transaction is finished
322322
await getFirstSentryEnvelopeRequest<Event>(page, url);

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,12 +297,12 @@ sentryTest(
297297
},
298298
);
299299

300-
sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => {
300+
sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestUrl, page }) => {
301301
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
302302
sentryTest.skip();
303303
}
304304

305-
const url = await getLocalTestPath({ testDir: __dirname });
305+
const url = await getLocalTestUrl({ testDir: __dirname });
306306

307307
const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
308308
const pageloadTraceContext = pageloadEvent.contexts?.trace;

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,12 @@ sentryTest(
294294
},
295295
);
296296

297-
sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => {
297+
sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestUrl, page }) => {
298298
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
299299
sentryTest.skip();
300300
}
301301

302-
const url = await getLocalTestPath({ testDir: __dirname });
302+
const url = await getLocalTestUrl({ testDir: __dirname });
303303

304304
const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
305305
const pageloadTraceContext = pageloadEvent.contexts?.trace;

dev-packages/browser-integration-tests/utils/fixtures.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import path from 'path';
33
/* eslint-disable no-empty-pattern */
44
import { test as base } from '@playwright/test';
55

6+
import { SDK_VERSION } from '@sentry/browser';
67
import { generatePage } from './generatePage';
78

89
export const TEST_HOST = 'http://sentry-test.io';
@@ -63,6 +64,17 @@ const sentryTest = base.extend<TestFixtures>({
6364

6465
return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue();
6566
});
67+
68+
// Ensure feedback can be lazy loaded
69+
await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback-modal.min.js`, route => {
70+
const filePath = path.resolve(testDir, './dist/feedback-modal.bundle.js');
71+
return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue();
72+
});
73+
74+
await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback-screenshot.min.js`, route => {
75+
const filePath = path.resolve(testDir, './dist/feedback-screenshot.bundle.js');
76+
return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue();
77+
});
6678
}
6779

6880
return pagePath;

dev-packages/browser-integration-tests/utils/generatePlugin.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
6363
bundle: 'build/bundles/[INTEGRATION_NAME].js',
6464
bundle_min: 'build/bundles/[INTEGRATION_NAME].min.js',
6565
},
66+
feedback: {
67+
bundle: 'build/bundles/[INTEGRATION_NAME].js',
68+
bundle_min: 'build/bundles/[INTEGRATION_NAME].min.js',
69+
},
6670
wasm: {
6771
cjs: 'build/npm/cjs/index.js',
6872
esm: 'build/npm/esm/index.js',
@@ -225,6 +229,24 @@ class SentryScenarioGenerationPlugin {
225229
.replace('_tracing', '')
226230
.replace('_feedback', '');
227231

232+
// For feedback bundle, make sure to add modal & screenshot integrations
233+
if (bundleKey.includes('_feedback')) {
234+
['feedback-modal', 'feedback-screenshot'].forEach(integration => {
235+
const fileName = `${integration}.bundle.js`;
236+
237+
// We add the files, but not a script tag - they are lazy-loaded
238+
addStaticAssetSymlink(
239+
this.localOutPath,
240+
path.resolve(
241+
PACKAGES_DIR,
242+
'feedback',
243+
BUNDLE_PATHS['feedback'][integrationBundleKey].replace('[INTEGRATION_NAME]', integration),
244+
),
245+
fileName,
246+
);
247+
});
248+
}
249+
228250
this.requiredIntegrations.forEach(integration => {
229251
const fileName = `${integration}.bundle.js`;
230252
addStaticAssetSymlink(

dev-packages/browser-integration-tests/utils/helpers.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,8 @@ export function shouldSkipTracingTest(): boolean {
248248
* @returns `true` if we should skip the feedback test
249249
*/
250250
export function shouldSkipFeedbackTest(): boolean {
251-
// TODO(fn): For now we are skipping feedback tests in all bundles, until we have a proper way to test them.
252-
// We need to make sure lazy loading works on release branches...
253251
const bundle = process.env.PW_BUNDLE as string | undefined;
254-
return !!bundle && bundle.startsWith('bundle');
252+
return bundle != null && !bundle.includes('feedback') && !bundle.includes('esm') && !bundle.includes('cjs');
255253
}
256254

257255
/**

0 commit comments

Comments
 (0)