Merge branch 'develop' into cg/fix-slow-test

Author: chargome

.github/workflows/build.yml

@@ -795,7 +795,8 @@ jobs:
     # - The build job was successful, not skipped
     # - AND if the profiling node bindings were either successful or skipped
     if: |
-      always() && needs.job_build.result == 'success' &&
+      always() &&
+      needs.job_build.result == 'success' &&
       (needs.job_compile_bindings_profiling_node.result == 'success' || needs.job_compile_bindings_profiling_node.result == 'skipped')
     needs: [job_get_metadata, job_build, job_compile_bindings_profiling_node]
     runs-on: ubuntu-20.04-large-js
@@ -981,13 +982,16 @@ jobs:
           directory: dev-packages/e2e-tests
           token: ${{ secrets.CODECOV_TOKEN }}
 
+  # - We skip optional tests on release branches
   job_optional_e2e_tests:
     name: E2E ${{ matrix.label || matrix.test-application }} Test
     # We only run E2E tests for non-fork PRs because the E2E tests require secrets to work and they can't be accessed from forks
     # We need to add the `always()` check here because the previous step has this as well :(
     # See: https://github.com/actions/runner/issues/2205
     if:
-      always() && needs.job_e2e_prepare.result == 'success' &&
+      always() &&
+      needs.job_get_metadata.outputs.is_release != 'true' &&
+      needs.job_e2e_prepare.result == 'success' &&
       needs.job_e2e_prepare.outputs.matrix-optional != '{"include":[]}' &&
       (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) &&
       github.actor != 'dependabot[bot]'

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

@@ -23,7 +23,7 @@ sentryTest('should capture feedback with custom button', async ({ getLocalTestUr
     }
   });
 
-  const url = await getLocalTestUrl({ testDir: __dirname });
+  const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
   await page.goto(url);
   await page.locator('#custom-feedback-button').click();

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

@@ -23,7 +23,7 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => {
     }
   });
 
-  const url = await getLocalTestUrl({ testDir: __dirname });
+  const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
   await page.goto(url);
   await page.getByText('Report a Bug').click();

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

@@ -31,7 +31,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestUrl
     }
   });
 
-  const url = await getLocalTestUrl({ testDir: __dirname });
+  const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
   await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);
 

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

@@ -23,7 +23,7 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => {
     }
   });
 
-  const url = await getLocalTestUrl({ testDir: __dirname });
+  const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
   await page.goto(url);
   await page.getByText('Report a Bug').click();

dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/test.ts

@@ -5,7 +5,7 @@ import { sentryTest } from '../../../../utils/fixtures';
 
 sentryTest('it allows to lazy load the feedback integration', async ({ getLocalTestUrl, page }) => {
   const bundle = process.env.PW_BUNDLE || '';
-  const url = await getLocalTestUrl({ testDir: __dirname });
+  const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
   await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback.min.js`, route => {
     return route.fulfill({

dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/template.html

@@ -6,6 +6,5 @@
   <body>
     <div id="content"></div>
     <img src="https://example.com/my/image.png" />
-    <button type="button">Test button</button>
   </body>
 </html>

dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts

@@ -5,7 +5,14 @@ import type { Event } from '@sentry/types';
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
 
-sentryTest('should capture a LCP vital with element details.', async ({ browserName, getLocalTestUrl, page }) => {
+/*
+  Because we "serve" the html test page as a static file, all requests for the image
+  are considered 3rd party requests. So the LCP value we obtain for the image is also
+  considered a 3rd party LCP value, meaning `renderTime` is only set if we also
+  return the `Timing-Allow-Origin` header.
+*/
+
+sentryTest('captures LCP vitals with element details.', async ({ browserName, getLocalTestUrl, page }) => {
   if (shouldSkipTracingTest() || browserName !== 'chromium') {
     sentryTest.skip();
   }
@@ -16,17 +23,50 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN
   });
 
   const url = await getLocalTestUrl({ testDir: __dirname });
-  const [eventData] = await Promise.all([
-    getFirstSentryEnvelopeRequest<Event>(page),
-    page.goto(url),
-    page.locator('button').click(),
-  ]);
+  const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest<Event>(page), page.goto(url)]);
 
   expect(eventData.measurements).toBeDefined();
   expect(eventData.measurements?.lcp?.value).toBeDefined();
 
-  // XXX: This should be body > img, but it can be flakey as sometimes it will report
-  // the button as LCP.
   expect(eventData.contexts?.trace?.data?.['lcp.element'].startsWith('body >')).toBe(true);
   expect(eventData.contexts?.trace?.data?.['lcp.size']).toBeGreaterThan(0);
+  expect(eventData.contexts?.trace?.data?.['lcp.loadTime']).toBeGreaterThan(0);
+
+  // renderTime is 0 because we don't return the `Timing-Allow-Origin` header
+  // and the image is loaded from a 3rd party origin
+  expect(eventData.contexts?.trace?.data?.['lcp.renderTime']).toBe(0);
+
+  // The LCP value should be the loadTime because the renderTime is not set
+  expect(eventData.measurements?.lcp?.value).toBeCloseTo(eventData.contexts?.trace?.data?.['lcp.loadTime']);
 });
+
+sentryTest(
+  'captures LCP renderTime when returning Timing-Allow-Origin header.',
+  async ({ browserName, getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest() || browserName !== 'chromium') {
+      sentryTest.skip();
+    }
+
+    page.route('**', route => route.continue());
+    page.route('**/my/image.png', async (route: Route) => {
+      return route.fulfill({
+        path: `${__dirname}/assets/sentry-logo-600x179.png`,
+        headers: { 'Timing-Allow-Origin': '*' },
+      });
+    });
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+    const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest<Event>(page), page.goto(url)]);
+
+    expect(eventData.measurements).toBeDefined();
+    expect(eventData.measurements?.lcp?.value).toBeDefined();
+
+    expect(eventData.contexts?.trace?.data?.['lcp.element'].startsWith('body >')).toBe(true);
+    expect(eventData.contexts?.trace?.data?.['lcp.size']).toBeGreaterThan(0);
+    expect(eventData.contexts?.trace?.data?.['lcp.loadTime']).toBeGreaterThan(0);
+    expect(eventData.contexts?.trace?.data?.['lcp.renderTime']).toBeGreaterThan(0);
+
+    // The LCP value should be the renderTime because the renderTime is set
+    expect(eventData.measurements?.lcp?.value).toBeCloseTo(eventData.contexts?.trace?.data?.['lcp.renderTime']);
+  },
+);

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

@@ -316,7 +316,7 @@ sentryTest(
       sentryTest.skip();
     }
 
-    const url = await getLocalTestUrl({ testDir: __dirname });
+    const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
     // ensure pageload transaction is finished
     await getFirstSentryEnvelopeRequest<Event>(page, url);

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

@@ -302,7 +302,7 @@ sentryTest('user feedback event after pageload has pageload traceId in headers',
     sentryTest.skip();
   }
 
-  const url = await getLocalTestUrl({ testDir: __dirname });
+  const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
 
   const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
   const pageloadTraceContext = pageloadEvent.contexts?.trace;