test(loader): Improve loader tests & update loader (#9245)

mydea · web-flow · commit 8d5c306198a9 · 2023-10-19T10:57:26.000+02:00
This updates the loader based on this PR: getsentry/sentry#58070 and also actually fixes/improves the loader tests by avoiding importing `@sentry/browser`. This is not needed and actually obfuscates the "real"/"normal" behavior (just look at the generated `dist` folder for a test. Instead, I added an eslint rule to allow `Sentry` as a global for these cases, and just access them directly. This also uncovered an incorrect test, which I adjusted so it works. I also added a new test to ensure custom config works.
diff --git a/.prettierignore b/.prettierignore
@@ -1,2 +1,3 @@
 *.md
 .nxcache
+packages/browser-integration-tests/fixtures/loader.js
diff --git a/packages/browser-integration-tests/.eslintrc.js b/packages/browser-integration-tests/.eslintrc.js
@@ -13,6 +13,14 @@ module.exports = {
     'fixtures/**',
     'tmp/**',
   ],
+  overrides: [
+    {
+      files: ['loader-suites/**/{subject,init}.js'],
+      globals: {
+        Sentry: true,
+      },
+    },
+  ],
   parserOptions: {
     sourceType: 'module',
   },
diff --git a/packages/browser-integration-tests/fixtures/loader.js b/packages/browser-integration-tests/fixtures/loader.js
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/init.js b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/init.js
@@ -1,3 +1 @@
-import * as Sentry from '@sentry/browser';
 
-window.Sentry = Sentry;
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/pageloadTransaction/init.js b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/pageloadTransaction/init.js
@@ -1,4 +1 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
 window._testBaseTimestamp = performance.timeOrigin / 1000;
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/init.js b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/init.js
@@ -1,3 +1 @@
-import * as Sentry from '@sentry/browser';
 
-window.Sentry = Sentry;
diff --git a/packages/browser-integration-tests/loader-suites/loader/noOnLoad/sdkLoadedInMeanwhile/subject.js b/packages/browser-integration-tests/loader-suites/loader/noOnLoad/sdkLoadedInMeanwhile/subject.js
@@ -3,7 +3,7 @@ setTimeout(() => {
   cdnScript.src = '/cdn.bundle.js';
 
   cdnScript.addEventListener('load', () => {
-    window.Sentry.init({
+    Sentry.init({
       dsn: 'https://public@dsn.ingest.sentry.io/1337',
       replaysSessionSampleRate: 0.42,
     });
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/captureExceptionInOnLoad/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/captureExceptionInOnLoad/init.js
@@ -1,7 +1,5 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
-
 Sentry.onLoad(function () {
+  // You _have_ to call Sentry.init() before calling Sentry.captureException() in Sentry.onLoad()!
+  Sentry.init();
   Sentry.captureException('Test exception');
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/captureExceptionInOnLoad/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/captureExceptionInOnLoad/test.ts
@@ -4,6 +4,14 @@ import { sentryTest } from '../../../../utils/fixtures';
 import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
 
 sentryTest('captureException works inside of onLoad', async ({ getLocalTestUrl, page }) => {
+  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 req = await waitForErrorRequestOnUrl(page, url);
 
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customBrowserTracing/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/customBrowserTracing/init.js
@@ -1,6 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
 window._testBaseTimestamp = performance.timeOrigin / 1000;
 
 Sentry.onLoad(function () {
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customInit/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/customInit/init.js
@@ -0,0 +1,20 @@
+window.__sentryOnLoad = 0;
+
+setTimeout(() => {
+  Sentry.onLoad(function () {
+    window.__hadSentry = window.sentryIsLoaded();
+
+    Sentry.init({
+      sampleRate: 0.5,
+    });
+
+    window.__sentryOnLoad++;
+  });
+});
+
+window.sentryIsLoaded = () => {
+  const __sentry = window.__SENTRY__;
+
+  // If there is a global __SENTRY__ that means that in any of the callbacks init() was already invoked
+  return !!(!(typeof __sentry === 'undefined') && __sentry.hub && __sentry.hub.getClient());
+};
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customInit/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/customInit/test.ts
@@ -0,0 +1,34 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { LOADER_CONFIGS } from '../../../../utils/generatePlugin';
+
+const bundle = process.env.PW_BUNDLE || '';
+const isLazy = LOADER_CONFIGS[bundle]?.lazy;
+
+sentryTest('always calls onLoad init correctly', async ({ getLocalTestUrl, page }) => {
+  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 });
+
+  await page.goto(url);
+
+  // We want to test that if we are _not_ lazy, we are correctly calling onLoad init()
+  // But if we are lazy and call `forceLoad`, we also call the onLoad init() correctly
+  if (isLazy) {
+    expect(await page.evaluate('window.__sentryOnLoad')).toEqual(0);
+    await page.evaluate('Sentry.forceLoad()');
+  }
+
+  await page.waitForFunction('window.__sentryOnLoad && window.sentryIsLoaded()');
+
+  expect(await page.evaluate('window.__hadSentry')).toEqual(false);
+  expect(await page.evaluate('window.__sentryOnLoad')).toEqual(1);
+  expect(await page.evaluate('Sentry.getCurrentHub().getClient().getOptions().sampleRate')).toEqual(0.5);
+});
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrations/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrations/init.js
@@ -1,7 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
-
 class CustomIntegration {
   constructor() {
     this.name = 'CustomIntegration';
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrationsFunction/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrationsFunction/init.js
@@ -1,7 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
-
 class CustomIntegration {
   constructor() {
     this.name = 'CustomIntegration';
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js
@@ -1,7 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
-
 Sentry.onLoad(function () {
   Sentry.init({
     integrations: [
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/init.js
@@ -1,7 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
-
 Sentry.onLoad(function () {
   Sentry.init({});
 });
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/onLoadLate/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/onLoadLate/init.js
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/onLoadLate/subject.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/onLoadLate/subject.js
@@ -0,0 +1,7 @@
+Sentry.forceLoad();
+
+setTimeout(() => {
+  Sentry.onLoad(function () {
+    Sentry.captureException('Test exception');
+  });
+}, 200);
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/onLoadLate/test.ts b/packages/browser-integration-tests/loader-suites/loader/onLoad/onLoadLate/test.ts
@@ -0,0 +1,21 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
+
+sentryTest('late onLoad call is handled', async ({ getLocalTestUrl, page }) => {
+  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 req = await waitForErrorRequestOnUrl(page, url);
+
+  const eventData = envelopeRequestParser(req);
+
+  expect(eventData.message).toBe('Test exception');
+});
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/pageloadTransaction/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/pageloadTransaction/init.js
@@ -1,6 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
 window._testBaseTimestamp = performance.timeOrigin / 1000;
 
 Sentry.onLoad(function () {
diff --git a/packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js b/packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js
@@ -1,7 +1,3 @@
-import * as Sentry from '@sentry/browser';
-
-window.Sentry = Sentry;
-
 Sentry.onLoad(function () {
   Sentry.init({});
 });

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`*.md`
`2`	`2`	`.nxcache`
	`3`	`+packages/browser-integration-tests/fixtures/loader.js`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1 @@`
`1`		`-import * as Sentry from '@sentry/browser';`
`2`	`1`
`3`		`-window.Sentry = Sentry;`