Skip to content

Commit 9a2942e

Browse files
logaretmclaude
andcommitted
ref(browser-tests): Address PR review feedback for waitForMetrics helper
Rename waitForMetricRequest to waitForMetrics since it returns SerializedMetric[] directly. Fix race conditions in navigation test by creating metric listener promises before triggering page.goto() and click() actions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5e34504 commit 9a2942e

File tree

2 files changed

+16
-10
lines changed
  • dev-packages/browser-integration-tests

2 files changed

+16
-10
lines changed

dev-packages/browser-integration-tests/suites/tracing/metrics/element-timing/test.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { Page, Route } from '@playwright/test';
22
import { expect } from '@playwright/test';
33
import type { SerializedMetric } from '@sentry/core';
44
import { sentryTest } from '../../../../utils/fixtures';
5-
import { shouldSkipMetricsTest, shouldSkipTracingTest, waitForMetricRequest } from '../../../../utils/helpers';
5+
import { shouldSkipMetricsTest, shouldSkipTracingTest, waitForMetrics } from '../../../../utils/helpers';
66

77
function getIdentifier(m: SerializedMetric): unknown {
88
return m.attributes?.['ui.element.identifier']?.value;
@@ -27,7 +27,7 @@ sentryTest(
2727

2828
// Wait for all expected element identifiers to arrive as metrics
2929
const [allMetrics] = await Promise.all([
30-
waitForMetricRequest(page, metrics => {
30+
waitForMetrics(page, metrics => {
3131
const seen = new Set(metrics.filter(m => m.name === 'ui.element.render_time').map(getIdentifier));
3232
return expectedIdentifiers.every(id => seen.has(id));
3333
}),
@@ -83,21 +83,27 @@ sentryTest('emits element timing metrics after navigation', async ({ getLocalTes
8383

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

86+
// Start listening before navigation to avoid missing metrics
87+
const pageloadMetricsPromise = waitForMetrics(page, metrics =>
88+
metrics.some(m => m.name === 'ui.element.render_time' && getIdentifier(m) === 'image-fast'),
89+
);
90+
8691
await page.goto(url);
8792

8893
// Wait for pageload element timing metrics to arrive before navigating
89-
await waitForMetricRequest(page, metrics =>
90-
metrics.some(m => m.name === 'ui.element.render_time' && getIdentifier(m) === 'image-fast'),
91-
);
94+
await pageloadMetricsPromise;
95+
96+
// Start listening before click to avoid missing metrics
97+
const navigationMetricsPromise = waitForMetrics(page, metrics => {
98+
const seen = new Set(metrics.filter(m => m.name === 'ui.element.render_time').map(getIdentifier));
99+
return seen.has('navigation-image') && seen.has('navigation-text');
100+
});
92101

93102
// Trigger navigation
94103
await page.locator('#button1').click();
95104

96105
// Wait for navigation element timing metrics
97-
const navigationMetrics = await waitForMetricRequest(page, metrics => {
98-
const seen = new Set(metrics.filter(m => m.name === 'ui.element.render_time').map(getIdentifier));
99-
return seen.has('navigation-image') && seen.has('navigation-text');
100-
});
106+
const navigationMetrics = await navigationMetricsPromise;
101107

102108
const renderTimeMetrics = navigationMetrics.filter(m => m.name === 'ui.element.render_time');
103109
const renderIdentifiers = renderTimeMetrics.map(getIdentifier);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ export function waitForClientReportRequest(page: Page, callback?: (report: Clien
290290
* and resolves when the callback returns true for the full set of collected metrics.
291291
* If no callback is provided, resolves on the first request containing metrics.
292292
*/
293-
export function waitForMetricRequest(
293+
export function waitForMetrics(
294294
page: Page,
295295
callback?: (metrics: SerializedMetric[]) => boolean,
296296
): Promise<SerializedMetric[]> {

0 commit comments

Comments
 (0)