From 8db2e15c8bfbd1733a678deb022469b54db3003b Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 21 Sep 2022 17:59:19 +0100 Subject: [PATCH 1/2] test(angular): Increase Angular SDK test coverage. --- packages/angular/.eslintrc.js | 1 + packages/angular/jest.config.js | 1 + packages/angular/package.json | 6 +- packages/angular/setup-jest.ts | 7 + packages/angular/test/tracing.test.ts | 424 ++++++++++++++++++++------ packages/angular/test/utils/index.ts | 96 ++++++ yarn.lock | 28 ++ 7 files changed, 463 insertions(+), 100 deletions(-) create mode 100644 packages/angular/setup-jest.ts create mode 100644 packages/angular/test/utils/index.ts diff --git a/packages/angular/.eslintrc.js b/packages/angular/.eslintrc.js index 46d8d10cc538..f60c8d708cf8 100644 --- a/packages/angular/.eslintrc.js +++ b/packages/angular/.eslintrc.js @@ -3,4 +3,5 @@ module.exports = { browser: true, }, extends: ['../../.eslintrc.js'], + ignorePatterns: ['setup-jest.ts'], }; diff --git a/packages/angular/jest.config.js b/packages/angular/jest.config.js index cd02790794a7..a61b7326ca54 100644 --- a/packages/angular/jest.config.js +++ b/packages/angular/jest.config.js @@ -3,4 +3,5 @@ const baseConfig = require('../../jest/jest.config.js'); module.exports = { ...baseConfig, testEnvironment: 'jsdom', + setupFilesAfterEnv: ['/setup-jest.ts'], }; diff --git a/packages/angular/package.json b/packages/angular/package.json index 586dec8f10fa..a787ba4b6c81 100644 --- a/packages/angular/package.json +++ b/packages/angular/package.json @@ -28,14 +28,18 @@ }, "devDependencies": { "@angular-devkit/build-angular": "~0.1002.4", + "@angular/animations": "~10.2.5", "@angular/cli": "^10.2.4", "@angular/common": "~10.2.5", "@angular/compiler": "^10.2.5", "@angular/compiler-cli": "~10.2.5", "@angular/core": "~10.2.5", + "@angular/platform-browser": "~10.2.5", + "@angular/platform-browser-dynamic": "~10.2.5", "@angular/router": "~10.2.5", "ng-packagr": "^10.1.0", - "typescript": "~4.0.2" + "typescript": "~4.0.2", + "zone.js": "^0.11.8" }, "scripts": { "build": "yarn build:ngc", diff --git a/packages/angular/setup-jest.ts b/packages/angular/setup-jest.ts new file mode 100644 index 000000000000..33cc915c7ff1 --- /dev/null +++ b/packages/angular/setup-jest.ts @@ -0,0 +1,7 @@ +import 'zone.js'; + +import { TestBed } from '@angular/core/testing'; +import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; + +TestBed.resetTestEnvironment(); +TestBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting()); diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index 69578f5e450d..dd2513148393 100644 --- a/packages/angular/test/tracing.test.ts +++ b/packages/angular/test/tracing.test.ts @@ -1,12 +1,21 @@ -import { ActivatedRouteSnapshot, Event, NavigationStart, ResolveEnd, Router } from '@angular/router'; -import { Hub, Transaction } from '@sentry/types'; -import { Subject } from 'rxjs'; +import { Component } from '@angular/core'; +import { ActivatedRouteSnapshot } from '@angular/router'; +import { Hub } from '@sentry/types'; -import { instrumentAngularRouting, TraceService } from '../src/index'; +import { instrumentAngularRouting, TraceClassDecorator, TraceDirective, TraceMethodDecorator } from '../src'; import { getParameterizedRouteFromSnapshot } from '../src/tracing'; +import { AppComponent, TestEnv } from './utils/index'; let transaction: any; -let customStartTransaction: (context: any) => Transaction | undefined; + +const defaultStartTransaction = (ctx: any) => { + transaction = { + ...ctx, + setName: jest.fn(name => (transaction.name = name)), + }; + + return transaction; +}; jest.mock('@sentry/browser', () => { const original = jest.requireActual('@sentry/browser'); @@ -27,11 +36,15 @@ jest.mock('@sentry/browser', () => { }); describe('Angular Tracing', () => { - const startTransaction = jest.fn(); + beforeEach(() => { + transaction = undefined; + }); describe('instrumentAngularRouting', () => { it('should attach the transaction source on the pageload transaction', () => { + const startTransaction = jest.fn(); instrumentAngularRouting(startTransaction); + expect(startTransaction).toHaveBeenCalledWith({ name: '/', op: 'pageload', @@ -40,71 +53,144 @@ describe('Angular Tracing', () => { }); }); + describe('getParameterizedRouteFromSnapshot', () => { + it.each([ + ['returns `/` empty object if the route no children', {}, '/'], + [ + 'returns the route of a snapshot without children', + { + firstChild: { routeConfig: { path: 'users/:id' } }, + }, + '/users/:id/', + ], + [ + 'returns the complete route of a snapshot with children', + { + firstChild: { + routeConfig: { path: 'orgs/:orgId' }, + firstChild: { + routeConfig: { path: 'projects/:projId' }, + firstChild: { routeConfig: { path: 'overview' } }, + }, + }, + }, + '/orgs/:orgId/projects/:projId/overview/', + ], + ])('%s', (_, routeSnapshot, expectedParams) => { + expect(getParameterizedRouteFromSnapshot(routeSnapshot as unknown as ActivatedRouteSnapshot)).toEqual( + expectedParams, + ); + }); + }); + describe('TraceService', () => { - let traceService: TraceService; - const routerEvents$: Subject = new Subject(); - const mockedRouter: Partial = { - events: routerEvents$, - }; + it('does not change the transaction name if the source is something other than `url`', async () => { + const customStartTransaction = jest.fn((ctx: any) => { + transaction = { + ...ctx, + metadata: { + ...ctx.metadata, + source: 'custom', + }, + setName: jest.fn(name => (transaction.name = name)), + }; - beforeEach(() => { - instrumentAngularRouting(startTransaction); - jest.resetAllMocks(); - traceService = new TraceService(mockedRouter as Router); - }); + return transaction; + }); - afterEach(() => { - traceService.ngOnDestroy(); - }); + const env = await TestEnv.setup({ + customStartTransaction, + routes: [ + { + path: '', + component: AppComponent, + }, + ], + }); - it('attaches the transaction source on a navigation change', () => { - routerEvents$.next(new NavigationStart(0, 'user/123/credentials')); + const url = '/'; - expect(startTransaction).toHaveBeenCalledTimes(1); - expect(startTransaction).toHaveBeenCalledWith({ - name: 'user/123/credentials', - op: 'navigation', + await env.navigateInAngular(url); + + expect(customStartTransaction).toHaveBeenCalledWith({ + name: url, + op: 'pageload', metadata: { source: 'url' }, }); + + expect(transaction.setName).toHaveBeenCalledTimes(0); + expect(transaction.name).toEqual(url); + + env.destroy(); }); - describe('URL parameterization', () => { - // TODO: These tests are real unit tests in the sense that they only test TraceService - // and we essentially just simulate a router navigation by firing the respective - // routing events and providing the raw URL + the resolved route parameters. - // In the future we should add more "wholesome" tests that let the Angular router - // do its thing (e.g. by calling router.navigate) and we check how our service - // reacts to it. - // Once we set up Jest for testing Angular, we can use TestBed to inject an actual - // router instance into TraceService and add more tests. - - beforeEach(() => { - transaction = undefined; - customStartTransaction = jest.fn((ctx: any) => { - transaction = { - ...ctx, - setName: jest.fn(name => (transaction.name = name)), - }; - return transaction; - }); + it('re-assigns routing span on navigation start with active transaction.', async () => { + const customStartTransaction = jest.fn(defaultStartTransaction); + + const env = await TestEnv.setup({ + customStartTransaction, }); + const finishMock = jest.fn(); + transaction.startChild = jest.fn(() => ({ + finish: finishMock, + })); + + await env.navigateInAngular('/'); + + expect(finishMock).toHaveBeenCalledTimes(1); + + env.destroy(); + }); + + it('finishes routing span on navigation end', async () => { + const customStartTransaction = jest.fn(defaultStartTransaction); + + const env = await TestEnv.setup({ + customStartTransaction, + }); + + const finishMock = jest.fn(); + transaction.startChild = jest.fn(() => ({ + finish: finishMock, + })); + + await env.navigateInAngular('/'); + + expect(finishMock).toHaveBeenCalledTimes(1); + + env.destroy(); + }); + + describe('URL parameterization', () => { it.each([ [ 'handles the root URL correctly', - '', + '/', { root: { firstChild: { routeConfig: null } }, }, '/', + [ + { + path: '', + component: AppComponent, + }, + ], ], [ 'does not alter static routes', - '/books/', + '/books', { root: { firstChild: { routeConfig: { path: 'books' } } }, }, '/books/', + [ + { + path: 'books', + component: AppComponent, + }, + ], ], [ 'parameterizes IDs in the URL', @@ -113,6 +199,12 @@ describe('Angular Tracing', () => { root: { firstChild: { routeConfig: { path: 'books/:bookId/details' } } }, }, '/books/:bookId/details/', + [ + { + path: 'books/:bookId/details', + component: AppComponent, + }, + ], ], [ 'parameterizes multiple IDs in the URL', @@ -121,6 +213,12 @@ describe('Angular Tracing', () => { root: { firstChild: { routeConfig: { path: 'org/:orgId/projects/:projId/events/:eventId' } } }, }, '/org/:orgId/projects/:projId/events/:eventId/', + [ + { + path: 'org/:orgId/projects/:projId/events/:eventId', + component: AppComponent, + }, + ], ], [ 'parameterizes URLs from route with child routes', @@ -137,81 +235,209 @@ describe('Angular Tracing', () => { }, }, '/org/:orgId/projects/:projId/events/:eventId/', + [ + { + path: 'org/:orgId', + component: AppComponent, + children: [ + { + path: 'projects/:projId', + component: AppComponent, + children: [ + { + path: 'events/:eventId', + component: AppComponent, + }, + ], + }, + ], + }, + ], ], - ])('%s and sets the source to `route`', (_, url, routerState, result) => { - instrumentAngularRouting(customStartTransaction, false, true); + ])('%s and sets the source to `route`', async (_, url, routerState, result, routes) => { + const customStartTransaction = jest.fn(defaultStartTransaction); + const env = await TestEnv.setup({ + customStartTransaction, + routes, + startTransactionOnPageLoad: false, + }); - // this event starts the transaction - routerEvents$.next(new NavigationStart(0, url)); + await env.navigateInAngular(url, routerState); expect(customStartTransaction).toHaveBeenCalledWith({ name: url, op: 'navigation', metadata: { source: 'url' }, }); + expect(transaction.setName).toHaveBeenCalledWith(result, 'route'); + + env.destroy(); + }); + }); + }); - // this event starts the parameterization - routerEvents$.next(new ResolveEnd(1, url, url, routerState as any)); + describe('TraceDirective', () => { + it('should create an instance', () => { + const directive = new TraceDirective(); + expect(directive).toBeTruthy(); + }); - expect(transaction.setName).toHaveBeenCalledWith(result, 'route'); + it('should create a child tracingSpan on init', async () => { + const directive = new TraceDirective(); + const customStartTransaction = jest.fn(defaultStartTransaction); + + const env = await TestEnv.setup({ + components: [TraceDirective], + customStartTransaction, + useTraceService: false, }); - it('does not change the transaction name if the source is something other than `url`', () => { - instrumentAngularRouting(customStartTransaction, false, true); + transaction.startChild = jest.fn(); - const url = '/user/12345/test'; + directive.ngOnInit(); - routerEvents$.next(new NavigationStart(0, url)); + expect(transaction.startChild).toHaveBeenCalledWith({ + op: 'ui.angular.init', + description: '', + }); - expect(customStartTransaction).toHaveBeenCalledWith({ - name: url, - op: 'navigation', - metadata: { source: 'url' }, - }); + env.destroy(); + }); - // Simulate that this transaction has a custom name: - transaction.metadata.source = 'custom'; + it('should finish tracingSpan after view init', async () => { + const directive = new TraceDirective(); + const finishMock = jest.fn(); + const customStartTransaction = jest.fn(defaultStartTransaction); - // this event starts the parameterization - routerEvents$.next( - new ResolveEnd(1, url, url, { - root: { firstChild: { routeConfig: { path: 'org/:orgId/projects/:projId/events/:eventId' } } }, - } as any), - ); + const env = await TestEnv.setup({ + components: [TraceDirective], + customStartTransaction, + useTraceService: false, + }); + + transaction.startChild = jest.fn(() => ({ + finish: finishMock, + })); - expect(transaction.setName).toHaveBeenCalledTimes(0); - expect(transaction.name).toEqual(url); + directive.ngOnInit(); + directive.ngAfterViewInit(); + + expect(finishMock).toHaveBeenCalledTimes(1); + + env.destroy(); + }); + }); + + describe('TraceClassDecorator', () => { + const origNgOnInitMock = jest.fn(); + const origNgAfterViewInitMock = jest.fn(); + + @Component({ + selector: 'layout-header', + template: '', + }) + @TraceClassDecorator() + class DecoratedComponent { + public ngOnInit() { + origNgOnInitMock(); + } + public ngAfterViewInit() { + origNgAfterViewInitMock(); + } + } + + it('Instruments `ngOnInit` and `ngAfterViewInit` methods of the decorated class', async () => { + const finishMock = jest.fn(); + const startChildMock = jest.fn(() => ({ + finish: finishMock, + })); + + const customStartTransaction = jest.fn((ctx: any) => { + transaction = { + ...ctx, + startChild: startChildMock, + }; + + return transaction; }); + + const env = await TestEnv.setup({ + customStartTransaction, + components: [DecoratedComponent], + defaultComponent: DecoratedComponent, + useTraceService: false, + }); + + expect(transaction.startChild).toHaveBeenCalledWith({ + description: '', + op: 'ui.angular.init', + }); + + expect(origNgOnInitMock).toHaveBeenCalledTimes(1); + expect(origNgAfterViewInitMock).toHaveBeenCalledTimes(1); + expect(finishMock).toHaveBeenCalledTimes(1); + + env.destroy(); }); }); - describe('getParameterizedRouteFromSnapshot', () => { - it.each([ - ['returns `/` empty object if the route no children', {}, '/'], - [ - 'returns the route of a snapshot without children', - { - firstChild: { routeConfig: { path: 'users/:id' } }, - }, - '/users/:id/', - ], - [ - 'returns the complete route of a snapshot with children', - { - firstChild: { - routeConfig: { path: 'orgs/:orgId' }, - firstChild: { - routeConfig: { path: 'projects/:projId' }, - firstChild: { routeConfig: { path: 'overview' } }, - }, - }, - }, - '/orgs/:orgId/projects/:projId/overview/', - ], - ])('%s', (_, routeSnapshot, expectedParams) => { - expect(getParameterizedRouteFromSnapshot(routeSnapshot as unknown as ActivatedRouteSnapshot)).toEqual( - expectedParams, - ); + describe('TraceMethodDecorator', () => { + const origNgOnInitMock = jest.fn(); + const origNgAfterViewInitMock = jest.fn(); + + @Component({ + selector: 'layout-header', + template: '', + }) + class DecoratedComponent { + @TraceMethodDecorator() + public ngOnInit() { + origNgOnInitMock(); + } + @TraceMethodDecorator() + public ngAfterViewInit() { + origNgAfterViewInitMock(); + } + } + + it('Instruments `ngOnInit` and `ngAfterViewInit` methods of the decorated class', async () => { + const startChildMock = jest.fn(); + + const customStartTransaction = jest.fn((ctx: any) => { + transaction = { + ...ctx, + startChild: startChildMock, + }; + + return transaction; + }); + + const env = await TestEnv.setup({ + customStartTransaction, + components: [DecoratedComponent], + defaultComponent: DecoratedComponent, + useTraceService: false, + }); + + expect(transaction.startChild).toHaveBeenCalledTimes(2); + expect(transaction.startChild.mock.calls[0][0]).toEqual({ + description: '', + op: 'ui.angular.ngOnInit', + startTimestamp: expect.any(Number), + endTimestamp: expect.any(Number), + }); + + expect(transaction.startChild.mock.calls[1][0]).toEqual({ + description: '', + op: 'ui.angular.ngAfterViewInit', + startTimestamp: expect.any(Number), + endTimestamp: expect.any(Number), + }); + + expect(origNgOnInitMock).toHaveBeenCalledTimes(1); + expect(origNgAfterViewInitMock).toHaveBeenCalledTimes(1); + + env.destroy(); }); }); }); diff --git a/packages/angular/test/utils/index.ts b/packages/angular/test/utils/index.ts new file mode 100644 index 000000000000..8788340d110f --- /dev/null +++ b/packages/angular/test/utils/index.ts @@ -0,0 +1,96 @@ +import { Component, NgModule } from '@angular/core'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { Router, Routes } from '@angular/router'; +import { RouterTestingModule } from '@angular/router/testing'; +import { Transaction } from '@sentry/types'; + +import { instrumentAngularRouting, TraceService } from '../../src'; + +@Component({ + template: '', +}) +export class AppComponent {} + +@NgModule({ + providers: [ + { + provide: TraceService, + deps: [Router], + }, + ], +}) +export class AppModule {} + +const defaultRoutes = [ + { + path: '', + component: AppComponent, + }, +]; + +export class TestEnv { + constructor( + public router: Router, + public fixture: ComponentFixture, + public traceService: TraceService | null, + ) { + fixture.detectChanges(); + } + + public static async setup(conf: { + routes?: Routes; + components?: any[]; + defaultComponent?: any; + customStartTransaction?: (context: any) => Transaction | undefined; + startTransactionOnPageLoad?: boolean; + startTransactionOnNavigation?: boolean; + useTraceService?: boolean; + }): Promise { + instrumentAngularRouting( + conf.customStartTransaction || jest.fn(), + conf.startTransactionOnPageLoad !== undefined ? conf.startTransactionOnPageLoad : true, + conf.startTransactionOnNavigation !== undefined ? conf.startTransactionOnNavigation : true, + ); + + const useTraceService = conf.useTraceService !== undefined ? conf.useTraceService : true; + const routes = conf.routes === undefined ? defaultRoutes : conf.routes; + + TestBed.configureTestingModule({ + imports: [AppModule, RouterTestingModule.withRoutes(routes)], + declarations: [...(conf.components || []), AppComponent], + providers: useTraceService + ? [ + { + provide: TraceService, + deps: [Router], + }, + ] + : [], + }); + + const router: Router = TestBed.inject(Router); + const traceService = useTraceService ? new TraceService(router) : null; + const fixture = TestBed.createComponent(conf.defaultComponent || AppComponent); + + return new TestEnv(router, fixture, traceService); + } + + public async navigateInAngular(url: string, routerState?: { [k: string]: any }): Promise { + return new Promise(resolve => { + return this.fixture.ngZone?.run(() => { + void this.router.navigateByUrl(url, { state: routerState }).then(() => { + this.fixture.detectChanges(); + resolve(); + }); + }); + }); + } + + public destroy(): void { + if (this.traceService) { + this.traceService.ngOnDestroy(); + } + + jest.clearAllMocks(); + } +} diff --git a/yarn.lock b/yarn.lock index 079c850f6b0d..2ab0aea06e3f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -128,6 +128,13 @@ ora "5.0.0" rxjs "6.6.2" +"@angular/animations@~10.2.5": + version "10.2.5" + resolved "https://registry.yarnpkg.com/@angular/animations/-/animations-10.2.5.tgz#9b4aaa2a2f88f848decb5a03f2ddaa2aed37642d" + integrity sha512-lIMwjY1pAqpCM4Ayndf2RsvOWRUc5QV7W82XNou6pIBv2T1i1XV6H72I5Sk9Z4sxxBYCWncEaEub+C6NcS8QRg== + dependencies: + tslib "^2.0.0" + "@angular/cli@^10.2.4": version "10.2.4" resolved "https://registry.yarnpkg.com/@angular/cli/-/cli-10.2.4.tgz#f8899eee8f774cd805b1831a8f2f865024e9f4e1" @@ -194,6 +201,20 @@ dependencies: tslib "^2.0.0" +"@angular/platform-browser-dynamic@~10.2.5": + version "10.2.5" + resolved "https://registry.yarnpkg.com/@angular/platform-browser-dynamic/-/platform-browser-dynamic-10.2.5.tgz#c9eea9e076a9fc8f80d7c041ba9766465613bb96" + integrity sha512-7z443I80K2CeqzczlSJ8BlABj0uRgnHUrABE8yLlU2BgifJrriBawzSXEV7UMEN7k7ezbc6NhpOn6Q6BrCKEOA== + dependencies: + tslib "^2.0.0" + +"@angular/platform-browser@~10.2.5": + version "10.2.5" + resolved "https://registry.yarnpkg.com/@angular/platform-browser/-/platform-browser-10.2.5.tgz#40dd88c937af7af56e3fb246608c7001e4ac09c7" + integrity sha512-3JDFRGNxr0IUkjSdGK2Q1BvqnSDpy9YWo0DJP+TEpgW578R84m4X7/wI3jJmFSC2yyouMWrHsot2vcBPAQj89g== + dependencies: + tslib "^2.0.0" + "@angular/router@~10.2.5": version "10.2.5" resolved "https://registry.yarnpkg.com/@angular/router/-/router-10.2.5.tgz#acc75a29ab0b54c8ebad7d2a896986a59d7d99ec" @@ -26984,3 +27005,10 @@ yocto-queue@^0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== + +zone.js@^0.11.8: + version "0.11.8" + resolved "https://registry.yarnpkg.com/zone.js/-/zone.js-0.11.8.tgz#40dea9adc1ad007b5effb2bfed17f350f1f46a21" + integrity sha512-82bctBg2hKcEJ21humWIkXRlLBBmrc3nN7DFh5LGGhcyycO2S7FN8NmdvlcKaGFDNVL4/9kFLmwmInTavdJERA== + dependencies: + tslib "^2.3.0" From 42323049e3ae630c4dffcbda830750df802c90b5 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 26 Sep 2022 12:54:50 +0100 Subject: [PATCH 2/2] Increase `errorhandler` test coverage. --- packages/angular/package.json | 1 - packages/angular/test/errorhandler.test.ts | 24 ++++++++++ packages/angular/test/tracing.test.ts | 54 ++++++++++++---------- packages/angular/test/utils/index.ts | 4 +- yarn.lock | 7 --- 5 files changed, 55 insertions(+), 35 deletions(-) diff --git a/packages/angular/package.json b/packages/angular/package.json index a787ba4b6c81..431b9d48690c 100644 --- a/packages/angular/package.json +++ b/packages/angular/package.json @@ -28,7 +28,6 @@ }, "devDependencies": { "@angular-devkit/build-angular": "~0.1002.4", - "@angular/animations": "~10.2.5", "@angular/cli": "^10.2.4", "@angular/common": "~10.2.5", "@angular/compiler": "^10.2.5", diff --git a/packages/angular/test/errorhandler.test.ts b/packages/angular/test/errorhandler.test.ts index 8c0059ae892b..324cbd0d1c7b 100644 --- a/packages/angular/test/errorhandler.test.ts +++ b/packages/angular/test/errorhandler.test.ts @@ -121,4 +121,28 @@ describe('SentryErrorHandler', () => { expect.any(Function), ); }); + + it('handleError method shows report dialog', () => { + const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog'); + + const errorHandler = createErrorHandler({ showDialog: true }); + errorHandler.handleError(new Error('test')); + + expect(showReportDialogSpy).toBeCalledTimes(1); + }); + + it('handleError method extracts error with a custom extractor', () => { + const customExtractor = (error: unknown) => { + if (typeof error === 'string') { + return new Error(`custom ${error}`); + } + return error; + }; + + const errorHandler = createErrorHandler({ extractor: customExtractor }); + errorHandler.handleError('error'); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(new Error('custom error'), expect.any(Function)); + }); }); diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index dd2513148393..d5a88c9bb082 100644 --- a/packages/angular/test/tracing.test.ts +++ b/packages/angular/test/tracing.test.ts @@ -120,6 +120,7 @@ describe('Angular Tracing', () => { expect(transaction.setName).toHaveBeenCalledTimes(0); expect(transaction.name).toEqual(url); + expect(transaction.metadata.source).toBe('custom'); env.destroy(); }); @@ -167,9 +168,6 @@ describe('Angular Tracing', () => { [ 'handles the root URL correctly', '/', - { - root: { firstChild: { routeConfig: null } }, - }, '/', [ { @@ -181,9 +179,6 @@ describe('Angular Tracing', () => { [ 'does not alter static routes', '/books', - { - root: { firstChild: { routeConfig: { path: 'books' } } }, - }, '/books/', [ { @@ -195,9 +190,6 @@ describe('Angular Tracing', () => { [ 'parameterizes IDs in the URL', '/books/1/details', - { - root: { firstChild: { routeConfig: { path: 'books/:bookId/details' } } }, - }, '/books/:bookId/details/', [ { @@ -209,9 +201,6 @@ describe('Angular Tracing', () => { [ 'parameterizes multiple IDs in the URL', '/org/sentry/projects/1234/events/04bc6846-4a1e-4af5-984a-003258f33e31', - { - root: { firstChild: { routeConfig: { path: 'org/:orgId/projects/:projId/events/:eventId' } } }, - }, '/org/:orgId/projects/:projId/events/:eventId/', [ { @@ -223,17 +212,6 @@ describe('Angular Tracing', () => { [ 'parameterizes URLs from route with child routes', '/org/sentry/projects/1234/events/04bc6846-4a1e-4af5-984a-003258f33e31', - { - root: { - firstChild: { - routeConfig: { path: 'org/:orgId' }, - firstChild: { - routeConfig: { path: 'projects/:projId' }, - firstChild: { routeConfig: { path: 'events/:eventId' } }, - }, - }, - }, - }, '/org/:orgId/projects/:projId/events/:eventId/', [ { @@ -254,7 +232,7 @@ describe('Angular Tracing', () => { }, ], ], - ])('%s and sets the source to `route`', async (_, url, routerState, result, routes) => { + ])('%s and sets the source to `route`', async (_, url, result, routes) => { const customStartTransaction = jest.fn(defaultStartTransaction); const env = await TestEnv.setup({ customStartTransaction, @@ -262,7 +240,7 @@ describe('Angular Tracing', () => { startTransactionOnPageLoad: false, }); - await env.navigateInAngular(url, routerState); + await env.navigateInAngular(url); expect(customStartTransaction).toHaveBeenCalledWith({ name: url, @@ -304,6 +282,32 @@ describe('Angular Tracing', () => { env.destroy(); }); + it('should use component name as span description', async () => { + const directive = new TraceDirective(); + const finishMock = jest.fn(); + const customStartTransaction = jest.fn(defaultStartTransaction); + + const env = await TestEnv.setup({ + components: [TraceDirective], + customStartTransaction, + useTraceService: false, + }); + + transaction.startChild = jest.fn(() => ({ + finish: finishMock, + })); + + directive.componentName = 'test-component'; + directive.ngOnInit(); + + expect(transaction.startChild).toHaveBeenCalledWith({ + op: 'ui.angular.init', + description: '', + }); + + env.destroy(); + }); + it('should finish tracingSpan after view init', async () => { const directive = new TraceDirective(); const finishMock = jest.fn(); diff --git a/packages/angular/test/utils/index.ts b/packages/angular/test/utils/index.ts index 8788340d110f..0f4ff55b0b23 100644 --- a/packages/angular/test/utils/index.ts +++ b/packages/angular/test/utils/index.ts @@ -75,10 +75,10 @@ export class TestEnv { return new TestEnv(router, fixture, traceService); } - public async navigateInAngular(url: string, routerState?: { [k: string]: any }): Promise { + public async navigateInAngular(url: string): Promise { return new Promise(resolve => { return this.fixture.ngZone?.run(() => { - void this.router.navigateByUrl(url, { state: routerState }).then(() => { + void this.router.navigateByUrl(url).then(() => { this.fixture.detectChanges(); resolve(); }); diff --git a/yarn.lock b/yarn.lock index 2ab0aea06e3f..c0fdf03f5997 100644 --- a/yarn.lock +++ b/yarn.lock @@ -128,13 +128,6 @@ ora "5.0.0" rxjs "6.6.2" -"@angular/animations@~10.2.5": - version "10.2.5" - resolved "https://registry.yarnpkg.com/@angular/animations/-/animations-10.2.5.tgz#9b4aaa2a2f88f848decb5a03f2ddaa2aed37642d" - integrity sha512-lIMwjY1pAqpCM4Ayndf2RsvOWRUc5QV7W82XNou6pIBv2T1i1XV6H72I5Sk9Z4sxxBYCWncEaEub+C6NcS8QRg== - dependencies: - tslib "^2.0.0" - "@angular/cli@^10.2.4": version "10.2.4" resolved "https://registry.yarnpkg.com/@angular/cli/-/cli-10.2.4.tgz#f8899eee8f774cd805b1831a8f2f865024e9f4e1"