From f0cf86cb932ad006735bc99a1111d7495a974037 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 2 Feb 2024 16:32:18 +0100 Subject: [PATCH] fix tests --- packages/vue/src/router.ts | 7 +- packages/vue/test/router.test.ts | 419 +++++++++++++++++++++++++++---- 2 files changed, 380 insertions(+), 46 deletions(-) diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index aba08e11f068..76bf4b1a443f 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -69,9 +69,9 @@ export function vueRouterInstrumentation( startTransaction({ name: WINDOW.location.pathname, op: 'pageload', - origin: 'auto.pageload.vue', attributes: { 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, }); @@ -151,7 +151,10 @@ export function instrumentVueRouter( } // Set router attributes on the existing pageload transaction // This will the origin, and add params & query attributes - pageloadTransaction.setAttributes(attributes); + pageloadTransaction.setAttributes({ + ...attributes, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', + }); } } diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 061bcdd3e1f9..a6d99211b663 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -1,9 +1,9 @@ import * as SentryBrowser from '@sentry/browser'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import type { Transaction } from '@sentry/types'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import type { SpanAttributes, Transaction } from '@sentry/types'; -import { vueRouterInstrumentation } from '../src'; -import type { Route } from '../src/router'; +import type { Route} from '../src/router'; +import { instrumentVueRouter, vueRouterInstrumentation } from '../src/router'; import * as vueTracing from '../src/tracing'; const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException'); @@ -13,7 +13,6 @@ const mockVueRouter = { beforeEach: jest.fn void) => void]>(), }; -const mockStartTransaction = jest.fn(); const mockNext = jest.fn(); const testRoutes: Record = { @@ -52,7 +51,10 @@ const testRoutes: Record = { }, }; +/* eslint-disable deprecation/deprecation */ describe('vueRouterInstrumentation()', () => { + const mockStartTransaction = jest.fn(); + afterEach(() => { jest.clearAllMocks(); }); @@ -101,16 +103,13 @@ describe('vueRouterInstrumentation()', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenCalledWith({ name: transactionName, - data: { + attributes: { + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: transactionSource, - params: to.params, - query: to.query, + ...getAttributesForRoute(to), }, op: 'navigation', - origin: 'auto.navigation.vue', - tags: { - 'routing.instrumentation': 'vue-router', - }, }); expect(mockNext).toHaveBeenCalledTimes(1); @@ -128,6 +127,7 @@ describe('vueRouterInstrumentation()', () => { updateName: jest.fn(), setData: jest.fn(), setAttribute: jest.fn(), + setAttributes: jest.fn(), metadata: {}, }; const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => { @@ -145,14 +145,12 @@ describe('vueRouterInstrumentation()', () => { expect(customMockStartTxn).toHaveBeenCalledTimes(1); expect(customMockStartTxn).toHaveBeenCalledWith({ name: '/', - data: { + attributes: { + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, op: 'pageload', - origin: 'auto.pageload.vue', - tags: { - 'routing.instrumentation': 'vue-router', - }, }); const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; @@ -165,8 +163,11 @@ describe('vueRouterInstrumentation()', () => { expect(mockedTxn.updateName).toHaveBeenCalledWith(transactionName); expect(mockedTxn.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, transactionSource); - expect(mockedTxn.setData).toHaveBeenNthCalledWith(1, 'params', to.params); - expect(mockedTxn.setData).toHaveBeenNthCalledWith(2, 'query', to.query); + expect(mockedTxn.setAttributes).toHaveBeenCalledWith({ + ...getAttributesForRoute(to), + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', + }); expect(mockNext).toHaveBeenCalledTimes(1); }, @@ -189,16 +190,13 @@ describe('vueRouterInstrumentation()', () => { // first startTx call happens when the instrumentation is initialized (for pageloads) expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/login', - data: { + attributes: { + 'routing.instrumentation': 'vue-router', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - params: to.params, - query: to.query, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', + ...getAttributesForRoute(to), }, op: 'navigation', - origin: 'auto.navigation.vue', - tags: { - 'routing.instrumentation': 'vue-router', - }, }); }); @@ -219,16 +217,13 @@ describe('vueRouterInstrumentation()', () => { // first startTx call happens when the instrumentation is initialized (for pageloads) expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: 'login-screen', - data: { + attributes: { + 'routing.instrumentation': 'vue-router', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', - params: to.params, - query: to.query, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', + ...getAttributesForRoute(to), }, op: 'navigation', - origin: 'auto.navigation.vue', - tags: { - 'routing.instrumentation': 'vue-router', - }, }); }); @@ -237,6 +232,7 @@ describe('vueRouterInstrumentation()', () => { updateName: jest.fn(), setData: jest.fn(), setAttribute: jest.fn(), + setAttributes: jest.fn(), name: '', toJSON: () => ({ data: { @@ -259,14 +255,12 @@ describe('vueRouterInstrumentation()', () => { expect(customMockStartTxn).toHaveBeenCalledTimes(1); expect(customMockStartTxn).toHaveBeenCalledWith({ name: '/', - data: { + attributes: { + 'routing.instrumentation': 'vue-router', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', }, op: 'pageload', - origin: 'auto.pageload.vue', - tags: { - 'routing.instrumentation': 'vue-router', - }, }); // now we give the transaction a custom name, thereby simulating what would @@ -278,13 +272,21 @@ describe('vueRouterInstrumentation()', () => { }, }); + const to = testRoutes['normalRoute1']; + const from = testRoutes['initialPageloadRoute']; + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; - beforeEachCallback(testRoutes['normalRoute1'], testRoutes['initialPageloadRoute'], mockNext); + beforeEachCallback(to, from, mockNext); expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); expect(mockedTxn.updateName).not.toHaveBeenCalled(); expect(mockedTxn.setAttribute).not.toHaveBeenCalled(); + expect(mockedTxn.setAttributes).toHaveBeenCalledWith({ + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', + ...getAttributesForRoute(to), + }); expect(mockedTxn.name).toEqual('customTxnName'); }); @@ -346,16 +348,345 @@ describe('vueRouterInstrumentation()', () => { // first startTx call happens when the instrumentation is initialized (for pageloads) expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/login', - data: { + attributes: { + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', + ...getAttributesForRoute(to), + }, + op: 'navigation', + }); + }); +}); +/* eslint-enable deprecation/deprecation */ + +describe('instrumentVueRouter()', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should return instrumentation that instruments VueRouter.onError', () => { + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + // check + expect(mockVueRouter.onError).toHaveBeenCalledTimes(1); + + const onErrorCallback = mockVueRouter.onError.mock.calls[0][0]; + + const testError = new Error(); + onErrorCallback(testError); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(testError, { mechanism: { handled: false } }); + }); + + it.each([ + ['normalRoute1', 'normalRoute2', '/accounts/:accountId', 'route'], + ['normalRoute2', 'namedRoute', 'login-screen', 'custom'], + ['normalRoute2', 'unmatchedRoute', '/e8733846-20ac-488c-9871-a5cbcb647294', 'url'], + ])( + 'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for navigations', + (fromKey, toKey, transactionName, transactionSource) => { + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + // check + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes[fromKey]; + const to = testRoutes[toKey]; + beforeEachCallback(to, from, mockNext); + + expect(mockStartSpan).toHaveBeenCalledTimes(1); + expect(mockStartSpan).toHaveBeenCalledWith({ + name: transactionName, + attributes: { + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: transactionSource, + ...getAttributesForRoute(to), + }, + op: 'navigation', + }); + + expect(mockNext).toHaveBeenCalledTimes(1); + }, + ); + + it.each([ + ['initialPageloadRoute', 'normalRoute1', '/books/:bookId/chapter/:chapterId', 'route'], + ['initialPageloadRoute', 'namedRoute', 'login-screen', 'custom'], + ['initialPageloadRoute', 'unmatchedRoute', '/e8733846-20ac-488c-9871-a5cbcb647294', 'url'], + ])( + 'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for pageloads', + (fromKey, toKey, transactionName, transactionSource) => { + const mockedTxn = { + updateName: jest.fn(), + setData: jest.fn(), + setAttribute: jest.fn(), + setAttributes: jest.fn(), + metadata: {}, + }; + + jest.spyOn(vueTracing, 'getActiveTransaction').mockImplementation(() => mockedTxn as unknown as Transaction); + + const mockStartSpan = jest.fn().mockImplementation(_ => { + return mockedTxn; + }); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + // no span is started for page load + expect(mockStartSpan).not.toHaveBeenCalled(); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes[fromKey]; + const to = testRoutes[toKey]; + + beforeEachCallback(to, from, mockNext); + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + expect(mockedTxn.updateName).toHaveBeenCalledWith(transactionName); + expect(mockedTxn.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, transactionSource); + expect(mockedTxn.setAttributes).toHaveBeenCalledWith({ + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', + ...getAttributesForRoute(to), + }); + + expect(mockNext).toHaveBeenCalledTimes(1); + }, + ); + + it('allows to configure routeLabel=path', () => { + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'path', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + // check + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes.normalRoute1; + const to = testRoutes.namedRoute; + beforeEachCallback(to, from, mockNext); + + // first startTx call happens when the instrumentation is initialized (for pageloads) + expect(mockStartSpan).toHaveBeenLastCalledWith({ + name: '/login', + attributes: { + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - params: to.params, - query: to.query, + ...getAttributesForRoute(to), }, op: 'navigation', - origin: 'auto.navigation.vue', - tags: { + }); + }); + + it('allows to configure routeLabel=name', () => { + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + // check + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes.normalRoute1; + const to = testRoutes.namedRoute; + beforeEachCallback(to, from, mockNext); + + // first startTx call happens when the instrumentation is initialized (for pageloads) + expect(mockStartSpan).toHaveBeenLastCalledWith({ + name: 'login-screen', + attributes: { 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', + ...getAttributesForRoute(to), }, + op: 'navigation', + }); + }); + + it("doesn't overwrite a pageload transaction name it was set to custom before the router resolved the route", () => { + const mockedTxn = { + updateName: jest.fn(), + setData: jest.fn(), + setAttribute: jest.fn(), + setAttributes: jest.fn(), + name: '', + toJSON: () => ({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + }, + }), + }; + const mockStartSpan = jest.fn().mockImplementation(_ => { + return mockedTxn; + }); + jest.spyOn(vueTracing, 'getActiveTransaction').mockImplementation(() => mockedTxn as unknown as Transaction); + + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + // check for transaction start + expect(mockStartSpan).not.toHaveBeenCalled(); + + // now we give the transaction a custom name, thereby simulating what would + // happen when users use the `beforeNavigate` hook + mockedTxn.name = 'customTxnName'; + mockedTxn.toJSON = () => ({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', + }, + }); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const to = testRoutes['normalRoute1']; + const from = testRoutes['initialPageloadRoute']; + + beforeEachCallback(to, from, mockNext); + + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + expect(mockedTxn.updateName).not.toHaveBeenCalled(); + expect(mockedTxn.setAttribute).not.toHaveBeenCalled(); + expect(mockedTxn.setAttributes).toHaveBeenCalledWith({ + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', + ...getAttributesForRoute(to), + }); + expect(mockedTxn.name).toEqual('customTxnName'); + }); + + test.each([ + [false, 0], + [true, 1], + ])( + 'should return instrumentation that considers the instrumentPageLoad = %p', + (instrumentPageLoad, expectedCallsAmount) => { + const mockedTxn = { + updateName: jest.fn(), + setData: jest.fn(), + setAttribute: jest.fn(), + setAttributes: jest.fn(), + name: '', + toJSON: () => ({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + }, + }), + }; + jest.spyOn(vueTracing, 'getActiveTransaction').mockImplementation(() => mockedTxn as unknown as Transaction); + + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad, instrumentNavigation: true }, + mockStartSpan, + ); + + // check + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + beforeEachCallback(testRoutes['normalRoute1'], testRoutes['initialPageloadRoute'], mockNext); + + expect(mockedTxn.updateName).toHaveBeenCalledTimes(expectedCallsAmount); + expect(mockStartSpan).not.toHaveBeenCalled(); + }, + ); + + test.each([ + [false, 0], + [true, 1], + ])( + 'should return instrumentation that considers the instrumentNavigation = %p', + (instrumentNavigation, expectedCallsAmount) => { + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation }, + mockStartSpan, + ); + + // check + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + beforeEachCallback(testRoutes['normalRoute2'], testRoutes['normalRoute1'], mockNext); + + expect(mockStartSpan).toHaveBeenCalledTimes(expectedCallsAmount); + }, + ); + + it("doesn't throw when `next` is not available in the beforeEach callback (Vue Router 4)", () => { + const mockStartSpan = jest.fn(); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'path', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes.normalRoute1; + const to = testRoutes.namedRoute; + beforeEachCallback(to, from, undefined); + + // first startTx call happens when the instrumentation is initialized (for pageloads) + expect(mockStartSpan).toHaveBeenLastCalledWith({ + name: '/login', + attributes: { + 'routing.instrumentation': 'vue-router', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + ...getAttributesForRoute(to), + }, + op: 'navigation', }); }); }); + +// Small helper function to get flattened attributes for test comparison +function getAttributesForRoute(route: Route): SpanAttributes { + const { params, query } = route; + + const attributes: SpanAttributes = {}; + + for (const key of Object.keys(params)) { + attributes[`params.${key}`] = params[key]; + } + for (const key of Object.keys(query)) { + const value = query[key]; + if (value) { + attributes[`query.${key}`] = value; + } + } + + return attributes; +}