From fc90cc4bbca71c283633e5e88fc251898b69ad8d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 1 Sep 2022 18:53:02 +0200 Subject: [PATCH 1/7] test(svelte): Add svelte testing library --- packages/svelte/jest.config.js | 4 ++++ packages/svelte/package.json | 4 +++- yarn.lock | 26 ++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/svelte/jest.config.js b/packages/svelte/jest.config.js index cd02790794a7..63637952d9d6 100644 --- a/packages/svelte/jest.config.js +++ b/packages/svelte/jest.config.js @@ -3,4 +3,8 @@ const baseConfig = require('../../jest/jest.config.js'); module.exports = { ...baseConfig, testEnvironment: 'jsdom', + transform: { + '^.+\\.svelte$': 'svelte-jester', + ...baseConfig.transform, + }, }; diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 0247f3ecf233..fba475b0aa6a 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -26,7 +26,9 @@ "svelte": "3.x" }, "devDependencies": { - "svelte": "3.49.0" + "@testing-library/svelte": "^3.2.1", + "svelte": "3.49.0", + "svelte-jester": "^2.3.2" }, "scripts": { "build": "run-p build:rollup build:types", diff --git a/yarn.lock b/yarn.lock index 7f25827379a4..544d08cb0d27 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4979,6 +4979,20 @@ dependencies: defer-to-connect "^1.0.1" +"@testing-library/dom@^8.1.0": + version "8.17.1" + resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-8.17.1.tgz#2d7af4ff6dad8d837630fecd08835aee08320ad7" + integrity sha512-KnH2MnJUzmFNPW6RIKfd+zf2Wue8mEKX0M3cpX6aKl5ZXrJM1/c/Pc8c2xDNYQCnJO48Sm5ITbMXgqTr3h4jxQ== + dependencies: + "@babel/code-frame" "^7.10.4" + "@babel/runtime" "^7.12.5" + "@types/aria-query" "^4.2.0" + aria-query "^5.0.0" + chalk "^4.1.0" + dom-accessibility-api "^0.5.9" + lz-string "^1.4.4" + pretty-format "^27.0.2" + "@testing-library/dom@^8.5.0": version "8.12.0" resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-8.12.0.tgz#fef5e545533fb084175dda6509ee71d7d2f72e23" @@ -5013,6 +5027,13 @@ "@testing-library/dom" "^8.5.0" "@types/react-dom" "*" +"@testing-library/svelte@^3.2.1": + version "3.2.1" + resolved "https://registry.yarnpkg.com/@testing-library/svelte/-/svelte-3.2.1.tgz#c63bd2b7df7907f26e91b4ce0c50c77d8e7c4745" + integrity sha512-qP5nMAx78zt+a3y9Sws9BNQYP30cOQ/LXDYuAj7wNtw86b7AtB7TFAz6/Av9hFsW3IJHPBBIGff6utVNyq+F1g== + dependencies: + "@testing-library/dom" "^8.1.0" + "@tootallnate/once@1": version "1.1.2" resolved "https://registry.yarnpkg.com/@tootallnate/once/-/once-1.1.2.tgz#ccb91445360179a04e7fe6aff78c00ffc1eeaf82" @@ -24945,6 +24966,11 @@ supports-preserve-symlinks-flag@^1.0.0: resolved "https://registry.yarnpkg.com/supports-preserve-symlinks-flag/-/supports-preserve-symlinks-flag-1.0.0.tgz#6eda4bd344a3c94aea376d4cc31bc77311039e09" integrity sha512-ot0WnXS9fgdkgIcePe6RHNk1WA8+muPa6cSjeR3V8K27q9BB1rTE3R1p7Hv0z1ZyAc8s6Vvv8DIyWf681MAt0w== +svelte-jester@^2.3.2: + version "2.3.2" + resolved "https://registry.yarnpkg.com/svelte-jester/-/svelte-jester-2.3.2.tgz#9eb818da30807bbcc940b6130d15b2c34408d64f" + integrity sha512-JtxSz4FWAaCRBXbPsh4LcDs4Ua7zdXgLC0TZvT1R56hRV0dymmNP+abw67DTPF7sQPyNxWsOKd0Sl7Q8SnP8kg== + svelte@3.49.0: version "3.49.0" resolved "https://registry.yarnpkg.com/svelte/-/svelte-3.49.0.tgz#5baee3c672306de1070c3b7888fc2204e36a4029" From dd187929f2d7f5335ded3e2fa056142d5ad59e87 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 1 Sep 2022 18:53:25 +0200 Subject: [PATCH 2/7] add component tracking test (svelte testing library) --- packages/svelte/test/performance.test.ts | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 packages/svelte/test/performance.test.ts diff --git a/packages/svelte/test/performance.test.ts b/packages/svelte/test/performance.test.ts new file mode 100644 index 000000000000..b5948577a819 --- /dev/null +++ b/packages/svelte/test/performance.test.ts @@ -0,0 +1,56 @@ +import { Scope } from '@sentry/hub'; +import { render } from '@testing-library/svelte'; + +import DummyComponent from './components/Dummy.svelte'; + +let testTransaction = { + startChild: jest.fn(), + finish: jest.fn(), +}; +let testUpdateSpan = { finish: jest.fn() }; +let testInitSpan = { finish: jest.fn(), startChild: jest.fn(), transaction: testTransaction }; + +jest.mock('@sentry/hub', () => { + const original = jest.requireActual('@sentry/hub'); + return { + ...original, + getCurrentHub(): { + getScope(): Scope; + } { + return { + getScope(): any { + return { + getTransaction: () => { + return testTransaction; + }, + }; + }, + }; + }, + }; +}); + +describe('Sentry.trackComponent()', () => { + beforeEach(() => { + jest.resetAllMocks(); + + testTransaction.startChild.mockReturnValue(testInitSpan); + testInitSpan.startChild.mockReturnValue(testUpdateSpan); + }); + it('creates nested init and update spans on component initialization', () => { + render(DummyComponent, { props: { options: {} } }); + + expect(testTransaction.startChild).toHaveBeenCalledWith({ + description: '', + op: 'ui.svelte.init', + }); + + expect(testInitSpan.startChild as jest.Mock).toHaveBeenCalledWith({ + description: '', + op: 'ui.svelte.update', + }); + + expect(testInitSpan.finish).toHaveBeenCalledTimes(1); + expect(testUpdateSpan.finish).toHaveBeenCalledTimes(1); + }); +}); From 62acf3d965357c58cd4adfea7d1cb2699bbb0796 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Sep 2022 10:25:38 +0200 Subject: [PATCH 3/7] add more trackComponent tests 100% line cov --- packages/svelte/test/performance.test.ts | 149 +++++++++++++++++++++-- 1 file changed, 142 insertions(+), 7 deletions(-) diff --git a/packages/svelte/test/performance.test.ts b/packages/svelte/test/performance.test.ts index b5948577a819..3e721f6ab198 100644 --- a/packages/svelte/test/performance.test.ts +++ b/packages/svelte/test/performance.test.ts @@ -1,14 +1,21 @@ import { Scope } from '@sentry/hub'; -import { render } from '@testing-library/svelte'; +import { act, render } from '@testing-library/svelte'; import DummyComponent from './components/Dummy.svelte'; -let testTransaction = { +let returnUndefinedTransaction = false; + +let testTransaction: { spans: any[]; startChild: jest.Mock; finish: jest.Mock } = { + spans: [], startChild: jest.fn(), finish: jest.fn(), }; let testUpdateSpan = { finish: jest.fn() }; -let testInitSpan = { finish: jest.fn(), startChild: jest.fn(), transaction: testTransaction }; +let testInitSpan: any = { + transaction: testTransaction, + finish: jest.fn(), + startChild: jest.fn(), +}; jest.mock('@sentry/hub', () => { const original = jest.requireActual('@sentry/hub'); @@ -21,7 +28,7 @@ jest.mock('@sentry/hub', () => { getScope(): any { return { getTransaction: () => { - return testTransaction; + return returnUndefinedTransaction ? undefined : testTransaction; }, }; }, @@ -33,10 +40,23 @@ jest.mock('@sentry/hub', () => { describe('Sentry.trackComponent()', () => { beforeEach(() => { jest.resetAllMocks(); + testTransaction.spans = []; - testTransaction.startChild.mockReturnValue(testInitSpan); - testInitSpan.startChild.mockReturnValue(testUpdateSpan); + testTransaction.startChild.mockImplementation(spanCtx => { + testTransaction.spans.push(spanCtx); + return testInitSpan; + }); + + testInitSpan.startChild.mockImplementation((spanCtx: any) => { + testTransaction.spans.push(spanCtx); + return testUpdateSpan; + }); + + testInitSpan.finish = jest.fn(); + testInitSpan.endTimestamp = undefined; + returnUndefinedTransaction = false; }); + it('creates nested init and update spans on component initialization', () => { render(DummyComponent, { props: { options: {} } }); @@ -45,12 +65,127 @@ describe('Sentry.trackComponent()', () => { op: 'ui.svelte.init', }); - expect(testInitSpan.startChild as jest.Mock).toHaveBeenCalledWith({ + expect(testInitSpan.startChild).toHaveBeenCalledWith({ + description: '', + op: 'ui.svelte.update', + }); + + expect(testInitSpan.finish).toHaveBeenCalledTimes(1); + expect(testUpdateSpan.finish).toHaveBeenCalledTimes(1); + expect(testTransaction.spans.length).toEqual(2); + }); + + it('creates an update span, when the component is updated', async () => { + // Make the finish() function actually end the initSpan + testInitSpan.finish.mockImplementation(() => { + testInitSpan.endTimestamp = new Date().getTime(); + }); + + // first we create the component + const { component } = render(DummyComponent, { props: { options: {} } }); + + // then trigger an update + // (just changing the trackUpdates prop so that we trigger an update. # + // The value doesn't do anything here) + await act(() => component.$set({ options: { trackUpdates: true } })); + + // once for init (unimportant here), once for starting the update span + expect(testTransaction.startChild).toHaveBeenCalledTimes(2); + expect(testTransaction.startChild).toHaveBeenLastCalledWith({ + description: '', + op: 'ui.svelte.update', + }); + expect(testTransaction.spans.length).toEqual(3); + }); + + it('only creates init spans if trackUpdates is deactivated', () => { + render(DummyComponent, { props: { options: { trackUpdates: false } } }); + + expect(testTransaction.startChild).toHaveBeenCalledWith({ + description: '', + op: 'ui.svelte.init', + }); + + expect(testInitSpan.startChild).not.toHaveBeenCalled(); + + expect(testInitSpan.finish).toHaveBeenCalledTimes(1); + expect(testTransaction.spans.length).toEqual(1); + }); + + it('only creates update spans if trackInit is deactivated', () => { + render(DummyComponent, { props: { options: { trackInit: false } } }); + + expect(testTransaction.startChild).toHaveBeenCalledWith({ description: '', op: 'ui.svelte.update', }); + expect(testInitSpan.startChild).not.toHaveBeenCalled(); + + expect(testInitSpan.finish).toHaveBeenCalledTimes(1); + expect(testTransaction.spans.length).toEqual(1); + }); + + it('creates no spans if trackInit and trackUpdates are deactivated', () => { + render(DummyComponent, { props: { options: { trackInit: false, trackUpdates: false } } }); + + expect(testTransaction.startChild).not.toHaveBeenCalled(); + expect(testInitSpan.startChild).not.toHaveBeenCalled(); + expect(testTransaction.spans.length).toEqual(0); + }); + + it('sets a custom component name as a span description if `componentName` is provided', async () => { + render(DummyComponent, { + props: { options: { componentName: 'CustomComponentName' } }, + }); + + expect(testTransaction.startChild).toHaveBeenCalledWith({ + description: '', + op: 'ui.svelte.init', + }); + + expect(testInitSpan.startChild).toHaveBeenCalledWith({ + description: '', + op: 'ui.svelte.update', + }); + expect(testInitSpan.finish).toHaveBeenCalledTimes(1); expect(testUpdateSpan.finish).toHaveBeenCalledTimes(1); + expect(testTransaction.spans.length).toEqual(2); + }); + + it("doesn't do anything, if there's no ongoing transaction", async () => { + returnUndefinedTransaction = true; + + render(DummyComponent, { + props: { options: { componentName: 'CustomComponentName' } }, + }); + + expect(testInitSpan.finish).toHaveBeenCalledTimes(0); + expect(testUpdateSpan.finish).toHaveBeenCalledTimes(0); + expect(testTransaction.spans.length).toEqual(0); + }); + + it("doesn't record update spans, if there's no ongoing transaction at that time", async () => { + // Make the finish() function actually end the initSpan + testInitSpan.finish.mockImplementation(() => { + testInitSpan.endTimestamp = new Date().getTime(); + }); + + // first we create the component + const { component } = render(DummyComponent, { props: { options: {} } }); + + // then clear the current transaction and trigger an update + returnUndefinedTransaction = true; + await act(() => component.$set({ options: { trackUpdates: true } })); + + // we should only record the init spans (including the initial update) + // but not the second update + expect(testTransaction.startChild).toHaveBeenCalledTimes(1); + expect(testTransaction.startChild).toHaveBeenLastCalledWith({ + description: '', + op: 'ui.svelte.init', + }); + expect(testTransaction.spans.length).toEqual(2); }); }); From 8e61acfd28a212dc9193ebc7ed6d0d0c5348ceb1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Sep 2022 11:03:32 +0200 Subject: [PATCH 4/7] fix linter errors --- packages/svelte/test/performance.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/test/performance.test.ts b/packages/svelte/test/performance.test.ts index 3e721f6ab198..d86902809aa4 100644 --- a/packages/svelte/test/performance.test.ts +++ b/packages/svelte/test/performance.test.ts @@ -5,13 +5,13 @@ import DummyComponent from './components/Dummy.svelte'; let returnUndefinedTransaction = false; -let testTransaction: { spans: any[]; startChild: jest.Mock; finish: jest.Mock } = { +const testTransaction: { spans: any[]; startChild: jest.Mock; finish: jest.Mock } = { spans: [], startChild: jest.fn(), finish: jest.fn(), }; -let testUpdateSpan = { finish: jest.fn() }; -let testInitSpan: any = { +const testUpdateSpan = { finish: jest.fn() }; +const testInitSpan: any = { transaction: testTransaction, finish: jest.fn(), startChild: jest.fn(), From 3efa16dd82d4e5fb62ca42ab12cc4bc795679ee9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Sep 2022 11:49:03 +0200 Subject: [PATCH 5/7] fix linter errors again (pls) --- packages/svelte/test/components/Dummy.svelte | 15 +++++++++++++++ packages/svelte/test/performance.test.ts | 1 + 2 files changed, 16 insertions(+) create mode 100644 packages/svelte/test/components/Dummy.svelte diff --git a/packages/svelte/test/components/Dummy.svelte b/packages/svelte/test/components/Dummy.svelte new file mode 100644 index 000000000000..a5fe608d9619 --- /dev/null +++ b/packages/svelte/test/components/Dummy.svelte @@ -0,0 +1,15 @@ + + +

Hi, I'm a dummy component for testing

diff --git a/packages/svelte/test/performance.test.ts b/packages/svelte/test/performance.test.ts index d86902809aa4..98dbfd59ba02 100644 --- a/packages/svelte/test/performance.test.ts +++ b/packages/svelte/test/performance.test.ts @@ -1,6 +1,7 @@ import { Scope } from '@sentry/hub'; import { act, render } from '@testing-library/svelte'; +// eslint-disable-next-line import/no-unresolved Eslint doesn't know about Svelte components import DummyComponent from './components/Dummy.svelte'; let returnUndefinedTransaction = false; From 30311efe952d964423c4df12df0d32c5a4037fb2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Sep 2022 11:51:28 +0200 Subject: [PATCH 6/7] remove commented code --- packages/svelte/test/components/Dummy.svelte | 4 ---- packages/svelte/test/performance.test.ts | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/svelte/test/components/Dummy.svelte b/packages/svelte/test/components/Dummy.svelte index a5fe608d9619..ef814473f6cf 100644 --- a/packages/svelte/test/components/Dummy.svelte +++ b/packages/svelte/test/components/Dummy.svelte @@ -6,10 +6,6 @@ export let options; Sentry.trackComponent(options); - - // onMount(() => console.log('>mount')); - // beforeUpdate(() => console.log('>beforeupdate')); - // afterUpdate(() => console.log('>afterupdate'));

Hi, I'm a dummy component for testing

diff --git a/packages/svelte/test/performance.test.ts b/packages/svelte/test/performance.test.ts index 98dbfd59ba02..e5ecae861a41 100644 --- a/packages/svelte/test/performance.test.ts +++ b/packages/svelte/test/performance.test.ts @@ -1,7 +1,8 @@ import { Scope } from '@sentry/hub'; import { act, render } from '@testing-library/svelte'; -// eslint-disable-next-line import/no-unresolved Eslint doesn't know about Svelte components +// linter doesn't like Svelte component imports +// eslint-disable-next-line import/no-unresolved import DummyComponent from './components/Dummy.svelte'; let returnUndefinedTransaction = false; From 7b9957fef11d23e0bd88257d7dbbc60b08bc8e50 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Sep 2022 12:30:07 +0200 Subject: [PATCH 7/7] only run svelte tests on Node >=10 --- scripts/test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/test.ts b/scripts/test.ts index 582a67732b55..a4904abe50f6 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -17,6 +17,7 @@ const NODE_8_SKIP_TESTS_PACKAGES = [ '@sentry/nextjs', '@sentry/angular', '@sentry/remix', + '@sentry/svelte', // svelte testing library requires Node >= 10 ]; // We have to downgrade some of our dependencies in order to run tests in Node 8 and 10.