From 8103e5cbb9b2efdf79d7e3b49184955b26633d29 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Thu, 27 May 2021 12:14:16 +0200 Subject: [PATCH 1/8] =?UTF-8?q?=E2=9C=85=20add=20expected=20behavior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/e2e/scenario/rum/init.scenario.ts | 99 ++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 15 deletions(-) diff --git a/test/e2e/scenario/rum/init.scenario.ts b/test/e2e/scenario/rum/init.scenario.ts index f9aeb3354d..97c0f1d122 100644 --- a/test/e2e/scenario/rum/init.scenario.ts +++ b/test/e2e/scenario/rum/init.scenario.ts @@ -1,8 +1,8 @@ -import { createTest } from '../../lib/framework' +import { createTest, EventRegistry } from '../../lib/framework' import { flushEvents } from '../../lib/helpers/sdk' -describe('before init API calls', () => { - createTest('should be associated to corresponding views') +describe('API calls and events around init', () => { + createTest('should be associated to corresponding views when views are automatically tracked') .withRum({ enableExperimentalFeatures: ['view-renaming'] }) .withRumInit((configuration) => { window.DD_RUM!.addError('before manual view') @@ -38,20 +38,89 @@ describe('before init API calls', () => { const documentEvent = events.rumResources.find((event) => event.resource.type === 'document')! expect(documentEvent.view.id).toBe(initialView.view.id) - const beforeManualViewError = events.rumErrors[0] - expect(beforeManualViewError.error.message).toBe('Provided "before manual view"') - expect(beforeManualViewError.view.id).toBe(initialView.view.id) + expectToHaveErrors( + events, + { message: 'Provided "before manual view"', viewId: initialView.view.id }, + { message: 'Provided "after manual view"', viewId: manualView.view.id } + ) - const afterManualViewError = events.rumErrors[1] - expect(afterManualViewError.error.message).toBe('Provided "after manual view"') - expect(afterManualViewError.view.id).toBe(manualView.view.id) + expectToHaveActions( + events, + { name: 'before manual view', viewId: initialView.view.id }, + { name: 'after manual view', viewId: manualView.view.id } + ) + }) + + createTest('should be associated to corresponding views when views are manually tracked') + .withRum({ trackViewsManually: true, enableExperimentalFeatures: ['view-renaming'] }) + .withRumInit((configuration) => { + window.DD_RUM!.addError('before init') + window.DD_RUM!.addAction('before init') + window.DD_RUM!.addTiming('before init') + + setTimeout(() => window.DD_RUM!.init(configuration), 10) + + setTimeout(() => { + window.DD_RUM!.addError('before manual view') + window.DD_RUM!.addAction('before manual view') + window.DD_RUM!.addTiming('before manual view') + }, 20) + + // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-return + setTimeout(() => (window.DD_RUM as any).startView('manual view'), 30) + + setTimeout(() => { + window.DD_RUM!.addError('after manual view') + window.DD_RUM!.addAction('after manual view') + window.DD_RUM!.addTiming('after manual view') + }, 40) + }) + .run(async ({ events }) => { + await flushEvents() + + const initialView = events.rumViews[0] + expect(initialView.view.name).toBe('manual view') + expect(initialView.view.custom_timings).toEqual({ + before_init: jasmine.any(Number), + before_manual_view: jasmine.any(Number), + after_manual_view: jasmine.any(Number), + }) - const beforeManualViewAction = events.rumActions[0] - expect(beforeManualViewAction.action.target!.name).toBe('before manual view') - expect(beforeManualViewAction.view.id).toBe(initialView.view.id) + const documentEvent = events.rumResources.find((event) => event.resource.type === 'document')! + expect(documentEvent.view.id).toBe(initialView.view.id) - const afterManualViewAction = events.rumActions[1] - expect(afterManualViewAction.action.target!.name).toBe('after manual view') - expect(afterManualViewAction.view.id).toBe(manualView.view.id) + expectToHaveErrors( + events, + { message: 'Provided "before init"', viewId: initialView.view.id }, + { message: 'Provided "before manual view"', viewId: initialView.view.id }, + { message: 'Provided "after manual view"', viewId: initialView.view.id } + ) + + expectToHaveActions( + events, + { name: 'before init', viewId: initialView.view.id }, + { name: 'before manual view', viewId: initialView.view.id }, + { name: 'after manual view', viewId: initialView.view.id } + ) }) }) + +function expectToHaveErrors(events: EventRegistry, ...errors: Array<{ message: string; viewId: string }>) { + expect(events.rumErrors.length).toBe(errors.length) + for (let i = 0; i < errors.length; i++) { + const registryError = events.rumErrors[i] + const expectedError = errors[i] + expect(registryError.error.message).toBe(expectedError.message) + expect(registryError.view.id).toBe(expectedError.viewId) + } +} + +function expectToHaveActions(events: EventRegistry, ...actions: Array<{ name: string; viewId: string }>) { + expect(events.rumActions.length).toBe(actions.length) + for (let i = 0; i < actions.length; i++) { + const registryAction = events.rumActions[i] + const expectedAction = actions[i] + expect(registryAction.action.target!.name).toBe(expectedAction.name) + expect(registryAction.view.id).toBe(expectedAction.viewId) + } +} From ef1b927876e51e6b2e010e7310f062f6a2fca1a6 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 2 Jun 2021 10:26:12 +0200 Subject: [PATCH 2/8] =?UTF-8?q?=E2=9C=A8=20prevent=20rum=20to=20start=20be?= =?UTF-8?q?fore=20the=20initial=20view=20start?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - introduce `trackViewsManually` configuration - on manual mode, wait for the first startView to start RUM - keep processing beforeInitCalls for potential startView but execute calls at start - ensure that recording start after RUM start --- packages/core/src/domain/configuration.ts | 6 + packages/rum-core/src/boot/rum.spec.ts | 2 +- packages/rum-core/src/boot/rum.ts | 26 ++-- .../rum-core/src/boot/rumPublicApi.spec.ts | 142 +++++++++++++++--- packages/rum-core/src/boot/rumPublicApi.ts | 95 +++++++----- .../view/viewCollection.spec.ts | 4 +- .../view/viewCollection.ts | 14 +- .../src/boot/rumRecorderPublicApi.spec.ts | 112 ++++++++++---- .../src/boot/rumRecorderPublicApi.ts | 80 ++++++---- 9 files changed, 349 insertions(+), 132 deletions(-) diff --git a/packages/core/src/domain/configuration.ts b/packages/core/src/domain/configuration.ts index c79de5f8d8..aba52a73a0 100644 --- a/packages/core/src/domain/configuration.ts +++ b/packages/core/src/domain/configuration.ts @@ -12,6 +12,7 @@ export const DEFAULT_CONFIGURATION = { sampleRate: 100, silentMultipleInit: false, trackInteractions: false, + trackViewsManually: false, /** * arbitrary value, byte precision not needed @@ -50,6 +51,7 @@ export interface UserConfiguration { enableExperimentalFeatures?: string[] silentMultipleInit?: boolean trackInteractions?: boolean + trackViewsManually?: boolean proxyHost?: string beforeSend?: (event: any) => void @@ -131,6 +133,10 @@ export function buildConfiguration(userConfiguration: UserConfiguration, buildEn configuration.trackInteractions = !!userConfiguration.trackInteractions } + if ('trackViewsManually' in userConfiguration) { + configuration.trackViewsManually = !!userConfiguration.trackViewsManually + } + return configuration } diff --git a/packages/rum-core/src/boot/rum.spec.ts b/packages/rum-core/src/boot/rum.spec.ts index 2a2cdeee10..f4ad900254 100644 --- a/packages/rum-core/src/boot/rum.spec.ts +++ b/packages/rum-core/src/boot/rum.spec.ts @@ -39,11 +39,11 @@ function startRum( ) const { stop: viewCollectionStop } = startViewCollection( lifeCycle, + configuration, location, domMutationObservable, foregroundContexts ) - return { stop: () => { rumEventCollectionStop() diff --git a/packages/rum-core/src/boot/rum.ts b/packages/rum-core/src/boot/rum.ts index 2ab996b8de..0a5d94486f 100644 --- a/packages/rum-core/src/boot/rum.ts +++ b/packages/rum-core/src/boot/rum.ts @@ -1,11 +1,11 @@ -import { combine, commonInit, Configuration } from '@datadog/browser-core' +import { combine, Configuration, InternalMonitoring } from '@datadog/browser-core' import { createDOMMutationObservable } from '../browser/domMutationObservable' import { startPerformanceCollection } from '../browser/performanceCollection' import { startRumAssembly } from '../domain/assembly' +import { startForegroundContexts } from '../domain/foregroundContexts' import { startInternalContext } from '../domain/internalContext' import { LifeCycle } from '../domain/lifeCycle' import { startParentContexts } from '../domain/parentContexts' -import { startForegroundContexts } from '../domain/foregroundContexts' import { startRequestCollection } from '../domain/requestCollection' import { startActionCollection } from '../domain/rumEventsCollection/action/actionCollection' import { startErrorCollection } from '../domain/rumEventsCollection/error/errorCollection' @@ -15,13 +15,16 @@ import { startViewCollection } from '../domain/rumEventsCollection/view/viewColl import { RumSession, startRumSession } from '../domain/rumSession' import { CommonContext } from '../rawRumEvent.types' import { startRumBatch } from '../transport/batch' - -import { buildEnv } from './buildEnv' import { RumUserConfiguration } from './rumPublicApi' -export function startRum(userConfiguration: RumUserConfiguration, getCommonContext: () => CommonContext) { +export function startRum( + userConfiguration: RumUserConfiguration, + configuration: Configuration, + internalMonitoring: InternalMonitoring, + getCommonContext: () => CommonContext, + initialViewName?: string +) { const lifeCycle = new LifeCycle() - const { configuration, internalMonitoring } = commonInit(userConfiguration, buildEnv) const session = startRumSession(configuration, lifeCycle) const domMutationObservable = createDOMMutationObservable() @@ -45,8 +48,14 @@ export function startRum(userConfiguration: RumUserConfiguration, getCommonConte startLongTaskCollection(lifeCycle) startResourceCollection(lifeCycle, session) - - const { addTiming, startView } = startViewCollection(lifeCycle, location, domMutationObservable, foregroundContexts) + const { addTiming, startView } = startViewCollection( + lifeCycle, + configuration, + location, + domMutationObservable, + foregroundContexts, + initialViewName + ) const { addError } = startErrorCollection(lifeCycle, configuration, foregroundContexts) const { addAction } = startActionCollection(lifeCycle, domMutationObservable, configuration, foregroundContexts) @@ -60,7 +69,6 @@ export function startRum(userConfiguration: RumUserConfiguration, getCommonConte addError, addTiming, startView, - configuration, lifeCycle, parentContexts, session, diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index c4baf9b49f..9073adf2eb 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -8,7 +8,6 @@ const noopStartRum = (): ReturnType => ({ addError: () => undefined, addTiming: () => undefined, startView: () => undefined, - configuration: { isEnabled: () => true } as any, getInternalContext: () => undefined, lifeCycle: {} as any, parentContexts: {} as any, @@ -448,19 +447,27 @@ describe('rum public api', () => { }) }) - describe('startView', () => { + describe('trackViews mode', () => { + const AUTO_CONFIGURATION = { ...DEFAULT_INIT_CONFIGURATION, enableExperimentalFeatures: ['view-renaming'] } + const MANUAL_CONFIGURATION = { ...AUTO_CONFIGURATION, trackViewsManually: true } + + let startRumSpy: jasmine.Spy let startViewSpy: jasmine.Spy['startView']> + let addTimingSpy: jasmine.Spy['addTiming']> let displaySpy: jasmine.Spy<() => void> let rumPublicApi: RumPublicApi let setupBuilder: TestSetupBuilder beforeEach(() => { - startViewSpy = jasmine.createSpy() + startViewSpy = jasmine.createSpy('startView') + addTimingSpy = jasmine.createSpy('addTiming') displaySpy = spyOn(display, 'error') - rumPublicApi = makeRumPublicApi(() => ({ + startRumSpy = jasmine.createSpy('startRum').and.returnValue({ ...noopStartRum(), + addTiming: addTimingSpy, startView: startViewSpy, - })) + }) + rumPublicApi = makeRumPublicApi(startRumSpy) setupBuilder = setup() }) @@ -468,34 +475,121 @@ describe('rum public api', () => { setupBuilder.cleanup() }) - it('should allow to start view before init', () => { - const { clock } = setupBuilder.withFakeClock().build() + describe('when auto', () => { + it('should start rum at init', () => { + rumPublicApi.init(AUTO_CONFIGURATION) - clock.tick(10) - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - ;(rumPublicApi as any).startView('foo') + expect(startRumSpy).toHaveBeenCalled() + }) - expect(startViewSpy).not.toHaveBeenCalled() + it('before init startView should be handled after init', () => { + const { clock } = setupBuilder.withFakeClock().build() - clock.tick(20) - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) + clock.tick(10) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('foo') + + expect(startViewSpy).not.toHaveBeenCalled() + + clock.tick(20) + rumPublicApi.init(AUTO_CONFIGURATION) + + expect(startViewSpy).toHaveBeenCalled() + expect(startViewSpy.calls.argsFor(0)[0]).toEqual('foo') + expect(startViewSpy.calls.argsFor(0)[1]).toEqual({ + relative: 10 as RelativeTime, + timeStamp: (jasmine.any(Number) as unknown) as TimeStamp, + }) + }) + + it('after init startView should be handle immediately', () => { + rumPublicApi.init(AUTO_CONFIGURATION) + + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('foo') - expect(startViewSpy.calls.argsFor(0)[0]).toEqual('foo') - expect(startViewSpy.calls.argsFor(0)[1]).toEqual({ - relative: 10 as RelativeTime, - timeStamp: (jasmine.any(Number) as unknown) as TimeStamp, + expect(startViewSpy).toHaveBeenCalled() + expect(startViewSpy.calls.argsFor(0)[0]).toEqual('foo') + expect(startViewSpy.calls.argsFor(0)[1]).toBeUndefined() + expect(displaySpy).not.toHaveBeenCalled() }) }) - it('should to start view', () => { - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) + describe('when views are tracked manually', () => { + it('should not start rum at init', () => { + rumPublicApi.init(MANUAL_CONFIGURATION) - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - ;(rumPublicApi as any).startView('foo') + expect(startRumSpy).not.toHaveBeenCalled() + }) - expect(startViewSpy.calls.argsFor(0)[0]).toEqual('foo') - expect(startViewSpy.calls.argsFor(0)[1]).toBeUndefined() - expect(displaySpy).not.toHaveBeenCalled() + it('before init startView should start rum', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('foo') + expect(startRumSpy).not.toHaveBeenCalled() + expect(startViewSpy).not.toHaveBeenCalled() + + rumPublicApi.init(MANUAL_CONFIGURATION) + expect(startRumSpy).toHaveBeenCalled() + expect(startRumSpy.calls.argsFor(0)[4]).toEqual('foo') + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('after init startView should start rum', () => { + rumPublicApi.init(MANUAL_CONFIGURATION) + expect(startRumSpy).not.toHaveBeenCalled() + expect(startViewSpy).not.toHaveBeenCalled() + + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('foo') + expect(startRumSpy).toHaveBeenCalled() + expect(startRumSpy.calls.argsFor(0)[4]).toEqual('foo') + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('after start rum startView should start view', () => { + rumPublicApi.init(MANUAL_CONFIGURATION) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('foo') + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('bar') + + expect(startRumSpy).toHaveBeenCalled() + expect(startRumSpy.calls.argsFor(0)[4]).toEqual('foo') + expect(startViewSpy).toHaveBeenCalled() + expect(startViewSpy.calls.argsFor(0)[0]).toEqual('bar') + expect(startViewSpy.calls.argsFor(0)[1]).toBeUndefined() + }) + + it('API calls should be handled in order', () => { + const { clock } = setupBuilder.withFakeClock().build() + + clock.tick(10) + rumPublicApi.addTiming('first') + + clock.tick(10) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumPublicApi as any).startView('foo') + + clock.tick(10) + rumPublicApi.addTiming('second') + + clock.tick(10) + rumPublicApi.init(MANUAL_CONFIGURATION) + + clock.tick(10) + rumPublicApi.addTiming('third') + + expect(addTimingSpy).toHaveBeenCalledTimes(3) + + expect(addTimingSpy.calls.argsFor(0)[0]).toEqual('first') + expect(addTimingSpy.calls.argsFor(0)[1]).toEqual(getTimeStamp(10 as RelativeTime)) + + expect(addTimingSpy.calls.argsFor(1)[0]).toEqual('second') + expect(addTimingSpy.calls.argsFor(1)[1]).toEqual(getTimeStamp(30 as RelativeTime)) + + expect(addTimingSpy.calls.argsFor(2)[0]).toEqual('third') + expect(addTimingSpy.calls.argsFor(2)[1]).toBeUndefined() // no time saved when started + }) }) }) }) diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 52dcb9cdb2..92d2e2fe51 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -14,10 +14,14 @@ import { clocksNow, timeStampNow, display, + commonInit, + Configuration, + InternalMonitoring, } from '@datadog/browser-core' import { ProvidedSource } from '../domain/rumEventsCollection/error/errorCollection' import { CommonContext, User, ActionType } from '../rawRumEvent.types' import { RumEvent } from '../rumEvent.types' +import { buildEnv } from './buildEnv' import { startRum } from './rum' export interface RumUserConfiguration extends UserConfiguration { @@ -29,7 +33,10 @@ export type RumPublicApi = ReturnType export type StartRum = ( userConfiguration: C, - getCommonContext: () => CommonContext + configuration: Configuration, + internalMonitoring: InternalMonitoring, + getCommonContext: () => CommonContext, + initialViewName?: string ) => StartRumResult type StartRumResult = ReturnType @@ -42,22 +49,18 @@ export function makeRumPublicApi(startRumImpl: S let getInternalContextStrategy: StartRumResult['getInternalContext'] = () => undefined - const beforeInitApiCalls = new BoundedBuffer() - let addTimingStrategy: StartRumResult['addTiming'] = (name) => { - const time = timeStampNow() - beforeInitApiCalls.add(() => addTimingStrategy(name, time)) + let bufferApiCalls = new BoundedBuffer() + let addTimingStrategy: StartRumResult['addTiming'] = (name, time = timeStampNow()) => { + bufferApiCalls.add(() => addTimingStrategy(name, time)) } - let startViewStrategy: StartRumResult['startView'] = (name) => { - const startClocks = clocksNow() - beforeInitApiCalls.add(() => startViewStrategy(name, startClocks)) + let startViewStrategy: StartRumResult['startView'] = (name, startClocks = clocksNow()) => { + bufferApiCalls.add(() => startViewStrategy(name, startClocks)) } - let addActionStrategy: StartRumResult['addAction'] = (action) => { - const commonContext = clonedCommonContext() - beforeInitApiCalls.add(() => addActionStrategy(action, commonContext)) + let addActionStrategy: StartRumResult['addAction'] = (action, commonContext = clonedCommonContext()) => { + bufferApiCalls.add(() => addActionStrategy(action, commonContext)) } - let addErrorStrategy: StartRumResult['addError'] = (providedError) => { - const commonContext = clonedCommonContext() - beforeInitApiCalls.add(() => addErrorStrategy(providedError, commonContext)) + let addErrorStrategy: StartRumResult['addError'] = (providedError, commonContext = clonedCommonContext()) => { + bufferApiCalls.add(() => addErrorStrategy(providedError, commonContext)) } function clonedCommonContext(): CommonContext { @@ -67,38 +70,62 @@ export function makeRumPublicApi(startRumImpl: S }) } - const rumPublicApi = makePublicApi({ - init: monitor((userConfiguration: C) => { - if ( - !checkCookiesAuthorized(buildCookieOptions(userConfiguration)) || - !checkIsNotLocalFile() || - !canInitRum(userConfiguration) - ) { - return - } - if (userConfiguration.publicApiKey) { - userConfiguration.clientToken = userConfiguration.publicApiKey + function initRum(userConfiguration: C) { + if ( + !checkCookiesAuthorized(buildCookieOptions(userConfiguration)) || + !checkIsNotLocalFile() || + !canInitRum(userConfiguration) + ) { + return + } + if (userConfiguration.publicApiKey) { + userConfiguration.clientToken = userConfiguration.publicApiKey + } + + const { configuration, internalMonitoring } = commonInit(userConfiguration, buildEnv) + if (!configuration.isEnabled('view-renaming') || !configuration.trackViewsManually) { + doStartRum() + } else { + // drain beforeInitCalls by buffering them until we start RUM + // if we get a startView, drain re-buffered calls before continuing to drain beforeInitCalls + // in order to ensure that calls are processed in order + const beforeInitCalls = bufferApiCalls + bufferApiCalls = new BoundedBuffer() + + startViewStrategy = (name) => { + doStartRum(name) } + beforeInitCalls.drain() + } + isAlreadyInitialized = true - let configuration + function doStartRum(initialViewName?: string) { let startView ;({ - configuration, startView, addAction: addActionStrategy, addError: addErrorStrategy, addTiming: addTimingStrategy, getInternalContext: getInternalContextStrategy, - } = startRumImpl(userConfiguration, () => ({ - user, - context: globalContextManager.get(), - }))) + } = startRumImpl( + userConfiguration, + configuration, + internalMonitoring, + () => ({ + user, + context: globalContextManager.get(), + }), + initialViewName + )) if (configuration.isEnabled('view-renaming')) { startViewStrategy = startView } - beforeInitApiCalls.drain() - isAlreadyInitialized = true - }), + bufferApiCalls.drain() + } + } + + const rumPublicApi = makePublicApi({ + init: monitor(initRum), addRumGlobalContext: monitor(globalContextManager.add), diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts index 99ec042720..2a589b1ef5 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts @@ -16,8 +16,8 @@ describe('viewCollection', () => { .withForegroundContexts({ getInForegroundPeriods: () => [{ start: 0 as ServerDuration, duration: 10 as ServerDuration }], }) - .beforeBuild(({ lifeCycle, foregroundContexts, domMutationObservable }) => { - startViewCollection(lifeCycle, location, domMutationObservable, foregroundContexts) + .beforeBuild(({ lifeCycle, configuration, foregroundContexts, domMutationObservable }) => { + startViewCollection(lifeCycle, configuration, location, domMutationObservable, foregroundContexts) }) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts index cc48f855fd..b797993910 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts @@ -1,4 +1,11 @@ -import { Duration, isEmptyObject, mapValues, ServerDuration, toServerDuration } from '@datadog/browser-core' +import { + Duration, + isEmptyObject, + mapValues, + ServerDuration, + toServerDuration, + Configuration, +} from '@datadog/browser-core' import { RawRumViewEvent, RumEventType } from '../../../rawRumEvent.types' import { LifeCycle, LifeCycleEventType } from '../../lifeCycle' import { DOMMutationObservable } from '../../../browser/domMutationObservable' @@ -7,9 +14,12 @@ import { trackViews, ViewEvent } from './trackViews' export function startViewCollection( lifeCycle: LifeCycle, + configuration: Configuration, location: Location, - domMutationObservable: DOMMutationObservable, + domMutationObservable: DOMMutationObservable, foregroundContexts: ForegroundContexts +, + initialViewName?: string ) { lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, (view) => lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, processViewUpdate(view, foregroundContexts)) diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts index 669cee37cf..425135d94f 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts @@ -12,11 +12,11 @@ describe('makeRumRecorderPublicApi', () => { let startRumSpy: jasmine.Spy beforeEach(() => { - stopRecordingSpy = jasmine.createSpy() - startRecordingSpy = jasmine.createSpy().and.callFake(() => ({ + stopRecordingSpy = jasmine.createSpy('stopRecording') + startRecordingSpy = jasmine.createSpy('startRecording').and.callFake(() => ({ stop: stopRecordingSpy, })) - startRumSpy = jasmine.createSpy().and.callFake((userConfiguration) => { + startRumSpy = jasmine.createSpy('startRum').and.callFake((userConfiguration) => { const configuration: Partial = { isEnabled(feature) { return includes(userConfiguration.enableExperimentalFeatures || [], feature) @@ -28,41 +28,95 @@ describe('makeRumRecorderPublicApi', () => { }) function getCommonContext() { - return startRumSpy.calls.first().args[1]() + return startRumSpy.calls.first().args[3]() } - describe('init', () => { - it('starts RUM when init is called', () => { - expect(startRumSpy).not.toHaveBeenCalled() - rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(startRumSpy).toHaveBeenCalled() - }) + describe('boot', () => { + describe('when tracking views automatically', () => { + it('starts RUM when init is called', () => { + expect(startRumSpy).not.toHaveBeenCalled() + rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION) + expect(startRumSpy).toHaveBeenCalled() + }) - it('starts recording when init() is called', () => { - expect(startRecordingSpy).not.toHaveBeenCalled() - rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(startRecordingSpy).toHaveBeenCalled() - }) + it('starts recording when init() is called', () => { + expect(startRecordingSpy).not.toHaveBeenCalled() + rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION) + expect(startRecordingSpy).toHaveBeenCalled() + }) - it('does not start recording when calling init() with manualSessionReplayRecordingStart: true', () => { - rumRecorderPublicApi.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true }) - expect(startRecordingSpy).not.toHaveBeenCalled() + it('does not start recording when calling init() with manualSessionReplayRecordingStart: true', () => { + rumRecorderPublicApi.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true }) + expect(startRecordingSpy).not.toHaveBeenCalled() + }) + + it('does not start recording when calling init() with the feature "postpone_start_recording"', () => { + rumRecorderPublicApi.init({ + ...DEFAULT_INIT_CONFIGURATION, + enableExperimentalFeatures: ['postpone_start_recording'], + }) + expect(startRecordingSpy).not.toHaveBeenCalled() + }) + + it('does not start recording before the page "load"', () => { + const { triggerOnLoad } = mockDocumentReadyState() + rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION) + expect(startRecordingSpy).not.toHaveBeenCalled() + triggerOnLoad() + expect(startRecordingSpy).toHaveBeenCalled() + }) }) - it('does not start recording when calling init() with the feature "postpone_start_recording"', () => { - rumRecorderPublicApi.init({ + describe('when tracking views manually', () => { + const MANUAL_VIEWS_CONFIGURATION = { ...DEFAULT_INIT_CONFIGURATION, - enableExperimentalFeatures: ['postpone_start_recording'], + enableExperimentalFeatures: ['view-renaming'], + trackViewsManually: true, + } + it('starts RUM when initial view is started', () => { + expect(startRumSpy).not.toHaveBeenCalled() + rumRecorderPublicApi.init(MANUAL_VIEWS_CONFIGURATION) + expect(startRumSpy).not.toHaveBeenCalled() + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumRecorderPublicApi as any).startView() + expect(startRumSpy).toHaveBeenCalled() }) - expect(startRecordingSpy).not.toHaveBeenCalled() - }) - it('does not start recording before the page "load"', () => { - const { triggerOnLoad } = mockDocumentReadyState() - rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(startRecordingSpy).not.toHaveBeenCalled() - triggerOnLoad() - expect(startRecordingSpy).toHaveBeenCalled() + it('starts recording when initial view is started', () => { + expect(startRecordingSpy).not.toHaveBeenCalled() + rumRecorderPublicApi.init(MANUAL_VIEWS_CONFIGURATION) + expect(startRecordingSpy).not.toHaveBeenCalled() + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumRecorderPublicApi as any).startView() + expect(startRecordingSpy).toHaveBeenCalled() + }) + + it('does not start recording when initial view is started with manualSessionReplayRecordingStart: true', () => { + rumRecorderPublicApi.init({ ...MANUAL_VIEWS_CONFIGURATION, manualSessionReplayRecordingStart: true }) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumRecorderPublicApi as any).startView() + expect(startRecordingSpy).not.toHaveBeenCalled() + }) + + it('does not start recording when initial view is started with the feature "postpone_start_recording"', () => { + rumRecorderPublicApi.init({ + ...MANUAL_VIEWS_CONFIGURATION, + enableExperimentalFeatures: ['postpone_start_recording'], + }) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumRecorderPublicApi as any).startView() + expect(startRecordingSpy).not.toHaveBeenCalled() + }) + + it('does not start recording before the page "load"', () => { + const { triggerOnLoad } = mockDocumentReadyState() + rumRecorderPublicApi.init(MANUAL_VIEWS_CONFIGURATION) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + ;(rumRecorderPublicApi as any).startView() + expect(startRecordingSpy).not.toHaveBeenCalled() + triggerOnLoad() + expect(startRecordingSpy).toHaveBeenCalled() + }) }) }) diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts index 69b337f542..8897bab9d3 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts @@ -1,5 +1,11 @@ -import { Configuration, monitor, noop, runOnReadyState } from '@datadog/browser-core' -import { LifeCycleEventType, makeRumPublicApi, RumUserConfiguration, StartRum } from '@datadog/browser-rum-core' +import { Configuration, monitor, noop, runOnReadyState, InternalMonitoring } from '@datadog/browser-core' +import { + LifeCycleEventType, + makeRumPublicApi, + RumUserConfiguration, + StartRum, + CommonContext, +} from '@datadog/browser-rum-core' import { startRecording } from './recorder' @@ -28,19 +34,48 @@ type RecorderState = } export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) { - const rumPublicApi = makeRumPublicApi((userConfiguration, getCommonContext) => { + let onRumStartStrategy = (userConfiguration: RumRecorderUserConfiguration, configuration: Configuration) => { + if ( + !userConfiguration.manualSessionReplayRecordingStart && + // TODO: remove this when no snippets without manualSessionReplayRecordingStart are served in + // the Datadog app. See RUMF-886 + !configuration.isEnabled('postpone_start_recording') + ) { + startSessionReplayRecordingStrategy() + } + } + let startSessionReplayRecordingStrategy = () => { + onRumStartStrategy = () => startSessionReplayRecordingStrategy() + } + let stopSessionReplayRecordingStrategy = () => { + onRumStartStrategy = noop + } + + function startRumRecorder( + userConfiguration: RumRecorderUserConfiguration, + configuration: Configuration, + internalMonitoring: InternalMonitoring, + getCommonContext: () => CommonContext, + initialViewName?: string + ) { let state: RecorderState = { status: RecorderStatus.Stopped, } - const startRumResult = startRumImpl(userConfiguration, () => ({ - ...getCommonContext(), - hasReplay: state.status === RecorderStatus.Started ? true : undefined, - })) + const startRumResult = startRumImpl( + userConfiguration, + configuration, + internalMonitoring, + () => ({ + ...getCommonContext(), + hasReplay: state.status === RecorderStatus.Started ? true : undefined, + }), + initialViewName + ) - const { lifeCycle, parentContexts, configuration, session } = startRumResult + const { lifeCycle, parentContexts, session } = startRumResult - startSessionReplayRecordingImpl = () => { + startSessionReplayRecordingStrategy = () => { if (state.status !== RecorderStatus.Stopped) { return } @@ -67,7 +102,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI }) } - stopSessionReplayRecordingImpl = () => { + stopSessionReplayRecordingStrategy = () => { if (state.status === RecorderStatus.Stopped) { return } @@ -82,37 +117,20 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) } - onInit(userConfiguration, configuration) + onRumStartStrategy(userConfiguration, configuration) return startRumResult - }) - - let onInit = (userConfiguration: RumRecorderUserConfiguration, configuration: Configuration) => { - if ( - !userConfiguration.manualSessionReplayRecordingStart && - // TODO: remove this when no snippets without manualSessionReplayRecordingStart are served in - // the Datadog app. See RUMF-886 - !configuration.isEnabled('postpone_start_recording') - ) { - startSessionReplayRecordingImpl() - } } - let startSessionReplayRecordingImpl = () => { - onInit = () => startSessionReplayRecordingImpl() - } - - let stopSessionReplayRecordingImpl = () => { - onInit = noop - } + const rumPublicApi = makeRumPublicApi(startRumRecorder) const rumRecorderPublicApi = { ...rumPublicApi, startSessionReplayRecording: monitor(() => { - startSessionReplayRecordingImpl() + startSessionReplayRecordingStrategy() }), stopSessionReplayRecording: monitor(() => { - stopSessionReplayRecordingImpl() + stopSessionReplayRecordingStrategy() }), } return rumRecorderPublicApi From afcb405afbef3e5ef5a46d9bf39a31d8c4af2a0b Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Thu, 3 Jun 2021 15:57:15 +0200 Subject: [PATCH 3/8] =?UTF-8?q?=E2=9C=A8=20allow=20to=20not=20track=20view?= =?UTF-8?q?s=20automatically?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit on manual mode: - do not create new view on location - do not track new view on session renewed - keep updating view location on location changes --- .../rumEventsCollection/view/trackViews.ts | 131 +++++++++++------- .../view/viewCollection.ts | 8 +- 2 files changed, 87 insertions(+), 52 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts index cdc0b9e948..00c3695249 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts @@ -54,54 +54,24 @@ export interface ViewEndedEvent { export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 export const SESSION_KEEP_ALIVE_INTERVAL = 5 * ONE_MINUTE -export function trackViews(location: Location, lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable) { +export function trackViews( + location: Location, + lifeCycle: LifeCycle, + domMutationObservable: DOMMutationObservable, + areViewsTrackedAutomatically: boolean, + initialViewName?: string +) { let isRecording = false - // eslint-disable-next-line prefer-const - let { stop: stopInitialViewTracking, initialView: currentView } = trackInitialView() - const { stop: stopLocationChangesTracking } = trackLocationChanges(() => { - if (areDifferentLocation(currentView.getLocation(), location)) { - // Renew view on location changes - currentView.end() - currentView.triggerUpdate() - currentView = trackViewChange() - return - } - currentView.updateLocation(location) - currentView.triggerUpdate() - }) - - // Renew view on session renewal - lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { - // do not trigger view update to avoid wrong data - currentView.end() - currentView = trackViewChange() - }) - - // End the current view on page unload - lifeCycle.subscribe(LifeCycleEventType.BEFORE_UNLOAD, () => { - currentView.end() - currentView.triggerUpdate() - }) - - lifeCycle.subscribe(LifeCycleEventType.RECORD_STARTED, () => { - isRecording = true - currentView.updateHasReplay(true) - }) - - lifeCycle.subscribe(LifeCycleEventType.RECORD_STOPPED, () => { - isRecording = false - }) + const { stop: stopInitialViewTracking, initialView } = trackInitialView(initialViewName) + let currentView = initialView - // Session keep alive - const keepAliveInterval = window.setInterval( - monitor(() => { - currentView.triggerUpdate() - }), - SESSION_KEEP_ALIVE_INTERVAL - ) + const { stop: stopViewLifeCycle } = startViewLifeCycle() + const { stop: stopViewCollectionMode } = areViewsTrackedAutomatically + ? startAutomaticViewCollection() + : startManualViewCollection() - function trackInitialView() { + function trackInitialView(name?: string) { const initialView = newView( lifeCycle, domMutationObservable, @@ -109,7 +79,8 @@ export function trackViews(location: Location, lifeCycle: LifeCycle, domMutation isRecording, ViewLoadingType.INITIAL_LOAD, document.referrer, - clocksOrigin() + clocksOrigin(), + name ) const { stop } = trackInitialViewTimings(lifeCycle, (timings) => { initialView.updateTimings(timings) @@ -131,6 +102,70 @@ export function trackViews(location: Location, lifeCycle: LifeCycle, domMutation ) } + function startViewLifeCycle() { + // End the current view on page unload + lifeCycle.subscribe(LifeCycleEventType.BEFORE_UNLOAD, () => { + currentView.end() + currentView.triggerUpdate() + }) + + lifeCycle.subscribe(LifeCycleEventType.RECORD_STARTED, () => { + isRecording = true + currentView.updateHasReplay(true) + }) + + lifeCycle.subscribe(LifeCycleEventType.RECORD_STOPPED, () => { + isRecording = false + }) + + // Session keep alive + const keepAliveInterval = window.setInterval( + monitor(() => { + currentView.triggerUpdate() + }), + SESSION_KEEP_ALIVE_INTERVAL + ) + + return { + stop: () => { + clearInterval(keepAliveInterval) + }, + } + } + + function startAutomaticViewCollection() { + lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { + // do not trigger view update to avoid wrong data + currentView.end() + // Renew view on session renewal + currentView = trackViewChange() + }) + + return trackLocationChanges(() => { + if (areDifferentLocation(currentView.getLocation(), location)) { + // Renew view on location changes + currentView.end() + currentView.triggerUpdate() + currentView = trackViewChange() + return + } + currentView.updateLocation(location) + currentView.triggerUpdate() + }) + } + + function startManualViewCollection() { + lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { + // do not trigger view update to avoid wrong data + currentView.end() + }) + + return trackLocationChanges(() => { + currentView.updateLocation(location) + currentView.triggerUpdate() + }) + } + return { addTiming: (name: string, time = timeStampNow()) => { currentView.addTiming(name, time) @@ -142,10 +177,10 @@ export function trackViews(location: Location, lifeCycle: LifeCycle, domMutation currentView = trackViewChange(startClocks, name) }, stop: () => { + stopViewCollectionMode() stopInitialViewTracking() - stopLocationChangesTracking() + stopViewLifeCycle() currentView.end() - clearInterval(keepAliveInterval) }, } } @@ -169,7 +204,7 @@ function newView( let location: Location = { ...initialLocation } let hasReplay = initialHasReplay - lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { id, startClocks, location, referrer }) + lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { id, name, startClocks, location, referrer }) // Update the view every time the measures are changing const { throttled: scheduleViewUpdate, cancel: cancelScheduleViewUpdate } = throttle( diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts index b797993910..dacc284bc9 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts @@ -16,16 +16,16 @@ export function startViewCollection( lifeCycle: LifeCycle, configuration: Configuration, location: Location, - domMutationObservable: DOMMutationObservable, - foregroundContexts: ForegroundContexts -, + domMutationObservable: DOMMutationObservable, + foregroundContexts: ForegroundContexts, initialViewName?: string ) { lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, (view) => lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, processViewUpdate(view, foregroundContexts)) ) - return trackViews(location, lifeCycle, domMutationObservable) + const shouldTrackViewsAutomatically = !configuration.isEnabled('view-renaming') || !configuration.trackViewsManually + return trackViews(location, lifeCycle, domMutationObservable, shouldTrackViewsAutomatically, initialViewName) } function processViewUpdate(view: ViewEvent, foregroundContexts: ForegroundContexts) { From 538ac38fbfb9ab25088b692ecf94e1e75c7bf7e2 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Thu, 3 Jun 2021 15:57:41 +0200 Subject: [PATCH 4/8] =?UTF-8?q?=E2=9C=85=20rework=20view=20unit=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - introduce viewTest helper to easily check view behaviors consistently - test behaviors for auto/manual mode - move location changes test to trackView.spec to be more symetric between auto and manual tests --- .../view/trackLocationChanges.spec.ts | 159 ---- .../view/trackViewMetrics.spec.ts | 200 ++-- .../view/trackViews.spec.ts | 896 +++++++++++------- packages/rum-core/test/specHelper.ts | 2 +- 4 files changed, 670 insertions(+), 587 deletions(-) delete mode 100644 packages/rum-core/src/domain/rumEventsCollection/view/trackLocationChanges.spec.ts diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackLocationChanges.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackLocationChanges.spec.ts deleted file mode 100644 index 3638fdf7b6..0000000000 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackLocationChanges.spec.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { LifeCycleEventType } from '@datadog/browser-rum-core' -import { TestSetupBuilder, setup } from '../../../../test/specHelper' -import { ViewCreatedEvent, trackViews } from './trackViews' - -function mockGetElementById() { - const fakeGetElementById = (elementId: string) => ((elementId === 'testHashValue') as any) as HTMLElement - return spyOn(document, 'getElementById').and.callFake(fakeGetElementById) -} - -describe('rum track location change', () => { - let setupBuilder: TestSetupBuilder - let initialViewId: string - let createSpy: jasmine.Spy<(event: ViewCreatedEvent) => void> - - beforeEach(() => { - setupBuilder = setup() - .withFakeLocation('/foo') - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - const subscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ id }) => { - initialViewId = id - subscription.unsubscribe() - }) - return trackViews(location, lifeCycle, domMutationObservable) - }) - createSpy = jasmine.createSpy('create') - }) - - afterEach(() => { - setupBuilder.cleanup() - }) - - it('should create new view on path change', () => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - history.pushState({}, '', '/bar') - - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.id).not.toEqual(initialViewId) - }) - - it('should create a new view on hash change from history', () => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - history.pushState({}, '', '/foo#bar') - - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.id).not.toEqual(initialViewId) - }) - - it('should not create a new view on hash change from history when the hash has kept the same value', () => { - history.pushState({}, '', '/foo#bar') - - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - history.pushState({}, '', '/foo#bar') - - expect(createSpy).not.toHaveBeenCalled() - }) - - it('should create a new view on hash change', (done) => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - function hashchangeCallBack() { - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.id).not.toEqual(initialViewId) - window.removeEventListener('hashchange', hashchangeCallBack) - done() - } - - window.addEventListener('hashchange', hashchangeCallBack) - - window.location.hash = '#bar' - }) - - it('should not create a new view when the hash has kept the same value', (done) => { - history.pushState({}, '', '/foo#bar') - - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - function hashchangeCallBack() { - expect(createSpy).not.toHaveBeenCalled() - window.removeEventListener('hashchange', hashchangeCallBack) - done() - } - - window.addEventListener('hashchange', hashchangeCallBack) - - window.location.hash = '#bar' - }) - - it('should not create a new view when it is an Anchor navigation', (done) => { - const { lifeCycle } = setupBuilder.build() - mockGetElementById() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - function hashchangeCallBack() { - expect(createSpy).not.toHaveBeenCalled() - window.removeEventListener('hashchange', hashchangeCallBack) - done() - } - - window.addEventListener('hashchange', hashchangeCallBack) - - window.location.hash = '#testHashValue' - }) - - it('should acknowledge the view location hash change after an Anchor navigation', (done) => { - const { lifeCycle } = setupBuilder.build() - const spyObj = mockGetElementById() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - function hashchangeCallBack() { - expect(createSpy).not.toHaveBeenCalled() - window.removeEventListener('hashchange', hashchangeCallBack) - - // clear mockGetElementById that fake Anchor nav - spyObj.and.callThrough() - - // This is not an Anchor nav anymore but the hash and pathname have not been updated - history.pushState({}, '', '/foo#testHashValue') - expect(createSpy).not.toHaveBeenCalled() - done() - } - - window.addEventListener('hashchange', hashchangeCallBack) - - window.location.hash = '#testHashValue' - }) - - it('should not create new view on search change', () => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - history.pushState({}, '', '/foo?bar=qux') - - expect(createSpy).not.toHaveBeenCalled() - }) - - it('should not create a new view when the search part of the hash changes', () => { - history.pushState({}, '', '/foo#bar') - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) - - history.pushState({}, '', '/foo#bar?search=1') - history.pushState({}, '', '/foo#bar?search=2') - history.pushState({}, '', '/foo#bar?') - history.pushState({}, '', '/foo#bar') - - expect(createSpy).not.toHaveBeenCalled() - }) -}) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 4654027982..054f3b2390 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -1,14 +1,15 @@ -import { LifeCycleEventType, RumEvent } from '@datadog/browser-rum-core' import { Context, RelativeTime, Duration } from '@datadog/browser-core' +import { LifeCycleEventType, RumEvent } from '@datadog/browser-rum-core' import { TestSetupBuilder, setup } from '../../../../test/specHelper' -import { RumEventType } from '../../../rawRumEvent.types' import { RumPerformanceNavigationTiming } from '../../../browser/performanceCollection' +import { RumEventType } from '../../../rawRumEvent.types' import { PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_MAX_DURATION, PAGE_ACTIVITY_VALIDATION_DELAY, } from '../../trackPageActivities' -import { ViewEvent, trackViews, THROTTLE_VIEW_UPDATE_PERIOD } from './trackViews' +import { THROTTLE_VIEW_UPDATE_PERIOD } from './trackViews' +import { setupViewTest, ViewTest } from './trackViews.spec' const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = (PAGE_ACTIVITY_VALIDATION_DELAY * 0.8) as Duration @@ -40,34 +41,16 @@ const FAKE_NAVIGATION_ENTRY_WITH_LOADEVENT_AFTER_ACTIVITY_TIMING: RumPerformance loadEventEnd: (BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY * 1.2) as RelativeTime, } -function spyOnViews() { - const handler = jasmine.createSpy() - - function getViewEvent(index: number) { - return handler.calls.argsFor(index)[0] as ViewEvent - } - - function getHandledCount() { - return handler.calls.count() - } - - return { handler, getViewEvent, getHandledCount } -} - describe('rum track view metrics', () => { let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getViewEvent: (index: number) => ViewEvent - let getHandledCount: () => number + let viewTest: ViewTest beforeEach(() => { - ;({ handler, getHandledCount, getViewEvent } = spyOnViews()) - setupBuilder = setup() .withFakeLocation('/foo') - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - return trackViews(location, lifeCycle, domMutationObservable) + .beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest }) }) @@ -82,41 +65,47 @@ describe('rum track view metrics', () => { it('should have an undefined loading time if there is no activity on a route change', () => { const { clock } = setupBuilder.build() + const { getViewUpdate, getViewUpdateCount, startView } = viewTest - history.pushState({}, '', '/bar') + startView() clock.tick(AFTER_PAGE_ACTIVITY_MAX_DURATION) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(2).loadingTime).toBeUndefined() + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(2).loadingTime).toBeUndefined() }) it('should have a loading time equal to the activity time if there is a unique activity on a route change', () => { const { domMutationObservable, clock } = setupBuilder.build() + const { getViewUpdate, startView } = viewTest - history.pushState({}, '', '/bar') + startView() clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) domMutationObservable.notify() clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) }) it('should use loadEventEnd for initial view when having no activity', () => { const { lifeCycle, clock } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) + const { getViewUpdate, getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).loadingTime).toEqual(FAKE_NAVIGATION_ENTRY.loadEventEnd) + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).loadingTime).toEqual(FAKE_NAVIGATION_ENTRY.loadEventEnd) }) it('should use loadEventEnd for initial view when load event is bigger than computed loading time', () => { const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) + const { getViewUpdate, getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) @@ -130,8 +119,8 @@ describe('rum track view metrics', () => { clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).loadingTime).toEqual( + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).loadingTime).toEqual( FAKE_NAVIGATION_ENTRY_WITH_LOADEVENT_AFTER_ACTIVITY_TIMING.loadEventEnd ) }) @@ -139,7 +128,9 @@ describe('rum track view metrics', () => { // eslint-disable-next-line max-len it('should use computed loading time for initial view when load event is smaller than computed loading time', () => { const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) + const { getViewUpdate, getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) lifeCycle.notify( @@ -150,90 +141,102 @@ describe('rum track view metrics', () => { clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) }) }) describe('event counts', () => { it('should track error count', () => { const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).eventCounts.errorCount).toEqual(0) + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).eventCounts.errorCount).toEqual(0) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.ERROR } as RumEvent & Context) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.ERROR } as RumEvent & Context) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).eventCounts.errorCount).toEqual(2) - expect(getViewEvent(2).eventCounts.errorCount).toEqual(0) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).eventCounts.errorCount).toEqual(2) + expect(getViewUpdate(2).eventCounts.errorCount).toEqual(0) }) it('should track long task count', () => { const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).eventCounts.longTaskCount).toEqual(0) + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).eventCounts.longTaskCount).toEqual(0) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.LONG_TASK } as RumEvent & Context) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).eventCounts.longTaskCount).toEqual(1) - expect(getViewEvent(2).eventCounts.longTaskCount).toEqual(0) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).eventCounts.longTaskCount).toEqual(1) + expect(getViewUpdate(2).eventCounts.longTaskCount).toEqual(0) }) it('should track resource count', () => { const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).eventCounts.resourceCount).toEqual(0) + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).eventCounts.resourceCount).toEqual(0) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE } as RumEvent & Context) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).eventCounts.resourceCount).toEqual(1) - expect(getViewEvent(2).eventCounts.resourceCount).toEqual(0) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).eventCounts.resourceCount).toEqual(1) + expect(getViewUpdate(2).eventCounts.resourceCount).toEqual(0) }) it('should track action count', () => { const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).eventCounts.userActionCount).toEqual(0) + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).eventCounts.userActionCount).toEqual(0) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.ACTION } as RumEvent & Context) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).eventCounts.userActionCount).toEqual(1) - expect(getViewEvent(2).eventCounts.userActionCount).toEqual(0) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).eventCounts.userActionCount).toEqual(1) + expect(getViewUpdate(2).eventCounts.userActionCount).toEqual(0) }) it('should reset event count when the view changes', () => { const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).eventCounts.resourceCount).toEqual(0) + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).eventCounts.resourceCount).toEqual(0) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE } as RumEvent & Context) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).eventCounts.resourceCount).toEqual(1) - expect(getViewEvent(2).eventCounts.resourceCount).toEqual(0) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).eventCounts.resourceCount).toEqual(1) + expect(getViewUpdate(2).eventCounts.resourceCount).toEqual(0) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE } as RumEvent & Context) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE } as RumEvent & Context) history.pushState({}, '', '/baz') - expect(getHandledCount()).toEqual(5) - expect(getViewEvent(3).eventCounts.resourceCount).toEqual(2) - expect(getViewEvent(4).eventCounts.resourceCount).toEqual(0) + expect(getViewUpdateCount()).toEqual(5) + expect(getViewUpdate(3).eventCounts.resourceCount).toEqual(2) + expect(getViewUpdate(4).eventCounts.resourceCount).toEqual(0) }) it('should update eventCounts when a resource event is collected (throttled)', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).eventCounts).toEqual({ + const { getViewUpdate, getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).eventCounts).toEqual({ errorCount: 0, longTaskCount: 0, resourceCount: 0, @@ -242,12 +245,12 @@ describe('rum track view metrics', () => { lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE } as RumEvent & Context) - expect(getHandledCount()).toEqual(1) + expect(getViewUpdateCount()).toEqual(1) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).eventCounts).toEqual({ + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).eventCounts).toEqual({ errorCount: 0, longTaskCount: 0, resourceCount: 1, @@ -257,21 +260,23 @@ describe('rum track view metrics', () => { it('should not update eventCounts after ending a view', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() - expect(getHandledCount()).toEqual(1) + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE } as RumEvent & Context) - expect(getHandledCount()).toEqual(1) + expect(getViewUpdateCount()).toEqual(1) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).id).toEqual(getViewEvent(0).id) - expect(getViewEvent(2).id).not.toEqual(getViewEvent(0).id) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).id).toEqual(getViewUpdate(0).id) + expect(getViewUpdate(2).id).not.toEqual(getViewUpdate(0).id) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(3) + expect(getViewUpdateCount()).toEqual(3) }) }) @@ -289,19 +294,24 @@ describe('rum track view metrics', () => { it('should be initialized to 0', () => { setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).cumulativeLayoutShift).toBe(0) + const { getViewUpdate, getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).cumulativeLayoutShift).toBe(0) }) it('should be initialized to undefined if layout-shift is not supported', () => { isLayoutShiftSupported = false setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).cumulativeLayoutShift).toBe(undefined) + const { getViewUpdate, getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).cumulativeLayoutShift).toBe(undefined) }) it('should accumulate layout shift values', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, getViewUpdateCount } = viewTest lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', @@ -317,12 +327,13 @@ describe('rum track view metrics', () => { clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).cumulativeLayoutShift).toBe(0.3) + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).cumulativeLayoutShift).toBe(0.3) }) it('should round the cumulative layout shift value to 4 decimals', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, getViewUpdateCount } = viewTest lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', @@ -338,12 +349,13 @@ describe('rum track view metrics', () => { clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).cumulativeLayoutShift).toBe(2.3457) + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).cumulativeLayoutShift).toBe(2.3457) }) it('should ignore entries with recent input', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, getViewUpdateCount } = viewTest lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', @@ -353,8 +365,8 @@ describe('rum track view metrics', () => { clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).cumulativeLayoutShift).toBe(0) + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).cumulativeLayoutShift).toBe(0) }) }) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts index 5e578e100c..3db351c2d6 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -1,13 +1,5 @@ -import { - ClocksState, - Duration, - RelativeTime, - TimeStamp, - timeStampNow, - display, - relativeToClocks, -} from '@datadog/browser-core' -import { setup, TestSetupBuilder } from '../../../../test/specHelper' +import { Duration, RelativeTime, timeStampNow, display, relativeToClocks } from '@datadog/browser-core' +import { setup, TestSetupBuilder, BuildContext } from '../../../../test/specHelper' import { RumLargestContentfulPaintTiming, RumPerformanceNavigationTiming, @@ -15,7 +7,7 @@ import { } from '../../../browser/performanceCollection' import { ViewLoadingType } from '../../../rawRumEvent.types' import { LifeCycleEventType } from '../../lifeCycle' -import { THROTTLE_VIEW_UPDATE_PERIOD, trackViews, ViewEvent, ViewCreatedEvent } from './trackViews' +import { THROTTLE_VIEW_UPDATE_PERIOD, trackViews, ViewEvent } from './trackViews' const FAKE_PAINT_ENTRY: RumPerformancePaintTiming = { entryType: 'paint', @@ -34,8 +26,40 @@ const FAKE_NAVIGATION_ENTRY: RumPerformanceNavigationTiming = { loadEventEnd: 567 as RelativeTime, } -function spyOnViews() { - const handler = jasmine.createSpy() +export type ViewTest = ReturnType + +export function setupViewTest( + { lifeCycle, location, domMutationObservable, configuration }: BuildContext, + initialViewName?: string +) { + const { handler: viewUpdateHandler, getViewEvent: getViewUpdate, getHandledCount: getViewUpdateCount } = spyOnViews( + 'view update' + ) + lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, viewUpdateHandler) + const { handler: viewCreateHandler, getViewEvent: getViewCreate, getHandledCount: getViewCreateCount } = spyOnViews( + 'view create' + ) + lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, viewCreateHandler) + const { stop, startView, addTiming } = trackViews( + location, + lifeCycle, + domMutationObservable, + !configuration.trackViewsManually, + initialViewName + ) + return { + stop, + startView, + addTiming, + getViewUpdate, + getViewUpdateCount, + getViewCreate, + getViewCreateCount, + } +} + +function spyOnViews(name?: string) { + const handler = jasmine.createSpy(name) function getViewEvent(index: number) { return handler.calls.argsFor(index)[0] as ViewEvent @@ -48,87 +72,223 @@ function spyOnViews() { return { handler, getViewEvent, getHandledCount } } -describe('rum view referrer', () => { +describe('track views automatically', () => { let setupBuilder: TestSetupBuilder - let initialViewCreatedEvent: ViewCreatedEvent - let createSpy: jasmine.Spy<(event: ViewCreatedEvent) => void> + let viewTest: ViewTest beforeEach(() => { setupBuilder = setup() .withFakeLocation('/foo') - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - const subscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, (event) => { - initialViewCreatedEvent = event - subscription.unsubscribe() - }) - return trackViews(location, lifeCycle, domMutationObservable) + .beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest }) - createSpy = jasmine.createSpy('create') }) afterEach(() => { setupBuilder.cleanup() }) - it('should set the document referrer as referrer for the initial view', () => { - setupBuilder.build() - expect(initialViewCreatedEvent.referrer).toEqual(document.referrer) - }) + describe('location changes', () => { + it('should update view location on search change', () => { + setupBuilder.build() + const { getViewCreateCount, getViewCreate, getViewUpdate, getViewUpdateCount } = viewTest - it('should set the previous view URL as referrer when a route change occurs', () => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) + history.pushState({}, '', '/foo?bar=qux') + + expect(getViewCreateCount()).toBe(1) + expect(getViewCreate(0).location.href).toMatch(/\/foo$/) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.location.href).toMatch(/\/foo\?bar=qux$/) + expect(lastUpdate.id).toBe(getViewCreate(0).id) + }) + + it('should create new view on path change', () => { + setupBuilder.build() + const { getViewCreateCount, getViewCreate } = viewTest + + expect(getViewCreateCount()).toBe(1) + expect(getViewCreate(0).location.href).toMatch(/\/foo$/) + + history.pushState({}, '', '/bar') + + expect(getViewCreateCount()).toBe(2) + expect(getViewCreate(1).location.href).toMatch(/\/bar$/) + }) + + it('should create new view on hash change from history', () => { + setupBuilder.build() + const { getViewCreateCount, getViewCreate } = viewTest + + expect(getViewCreateCount()).toBe(1) + expect(getViewCreate(0).location.href).toMatch(/\/foo$/) + + history.pushState({}, '', '/foo#bar') + + expect(getViewCreateCount()).toBe(2) + expect(getViewCreate(1).location.href).toMatch(/\/foo#bar$/) + }) + + it('should not create a new view on hash change from history when the hash has kept the same value', () => { + history.pushState({}, '', '/foo#bar') + + setupBuilder.build() + const { getViewCreateCount } = viewTest + + expect(getViewCreateCount()).toBe(1) + + history.pushState({}, '', '/foo#bar') + + expect(getViewCreateCount()).toBe(1) + }) + + it('should create a new view on hash change', (done) => { + setupBuilder.build() + const { getViewCreateCount } = viewTest + + function hashchangeCallBack() { + expect(getViewCreateCount()).toBe(2) + window.removeEventListener('hashchange', hashchangeCallBack) + done() + } + + window.addEventListener('hashchange', hashchangeCallBack) + + expect(getViewCreateCount()).toBe(1) + window.location.hash = '#bar' + }) + + it('should not create a new view when the hash has kept the same value', (done) => { + history.pushState({}, '', '/foo#bar') + + setupBuilder.build() + const { getViewCreateCount } = viewTest + + function hashchangeCallBack() { + expect(getViewCreateCount()).toBe(1) + window.removeEventListener('hashchange', hashchangeCallBack) + done() + } + + window.addEventListener('hashchange', hashchangeCallBack) + + expect(getViewCreateCount()).toBe(1) + window.location.hash = '#bar' + }) - history.pushState({}, '', '/bar') + function mockGetElementById() { + const fakeGetElementById = (elementId: string) => ((elementId === 'testHashValue') as any) as HTMLElement + return spyOn(document, 'getElementById').and.callFake(fakeGetElementById) + } - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.referrer).toEqual(jasmine.stringMatching(/\/foo$/)) + it('should not create a new view when it is an Anchor navigation', (done) => { + setupBuilder.build() + const { getViewCreateCount } = viewTest + mockGetElementById() + + function hashchangeCallBack() { + expect(getViewCreateCount()).toBe(1) + window.removeEventListener('hashchange', hashchangeCallBack) + done() + } + + window.addEventListener('hashchange', hashchangeCallBack) + + expect(getViewCreateCount()).toBe(1) + window.location.hash = '#testHashValue' + }) + + it('should not create a new view when the search part of the hash changes', () => { + history.pushState({}, '', '/foo#bar') + setupBuilder.build() + const { getViewCreateCount } = viewTest + + expect(getViewCreateCount()).toBe(1) + + history.pushState({}, '', '/foo#bar?search=1') + history.pushState({}, '', '/foo#bar?search=2') + history.pushState({}, '', '/foo#bar?') + history.pushState({}, '', '/foo#bar') + + expect(getViewCreateCount()).toBe(1) + }) }) - it('should set the previous view URL as referrer when a the session is renewed', () => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) + describe('renew session', () => { + it('should create new view on renew session', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewCreateCount } = viewTest + + expect(getViewCreateCount()).toBe(1) + + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + expect(getViewCreateCount()).toBe(2) + }) - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + it('should not update the current view when the session is renewed', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdateCount, getViewUpdate } = viewTest - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.referrer).toEqual(jasmine.stringMatching(/\/foo$/)) + expect(getViewUpdateCount()).toEqual(1) + + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(0).id).not.toBe(getViewUpdate(1).id) + }) }) - it('should use the most up-to-date URL of the previous view as a referrer', () => { - const { lifeCycle } = setupBuilder.build() - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) + describe('view referrer', () => { + it('should set the document referrer as referrer for the initial view', () => { + setupBuilder.build() + const { getViewCreate } = viewTest + + expect(getViewCreate(0).referrer).toEqual(document.referrer) + }) + + it('should set the previous view URL as referrer when a route change occurs', () => { + setupBuilder.build() + const { getViewCreate } = viewTest + + history.pushState({}, '', '/bar') + + expect(getViewCreate(1).referrer).toEqual(jasmine.stringMatching(/\/foo$/)) + }) + + it('should set the previous view URL as referrer when a the session is renewed', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewCreate } = viewTest + + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + expect(getViewCreate(1).referrer).toEqual(jasmine.stringMatching(/\/foo$/)) + }) - history.pushState({}, '', '/foo?a=b') - history.pushState({}, '', '/bar') + it('should use the most up-to-date URL of the previous view as a referrer', () => { + setupBuilder.build() + const { getViewCreate } = viewTest - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.referrer).toEqual(jasmine.stringMatching(/\/foo\?a=b$/)) + history.pushState({}, '', '/foo?a=b') + history.pushState({}, '', '/bar') + + expect(getViewCreate(1).referrer).toEqual(jasmine.stringMatching(/\/foo\?a=b$/)) + }) }) }) -describe('rum track renew session', () => { +describe('track views manually', () => { let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let initialViewId: string - let getHandledCount: () => number - let getViewEvent: (index: number) => ViewEvent + let viewTest: ViewTest beforeEach(() => { - ;({ handler, getViewEvent, getHandledCount } = spyOnViews()) - setupBuilder = setup() .withFakeLocation('/foo') - .beforeBuild(({ lifeCycle, location, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - const subscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ id }) => { - initialViewId = id - subscription.unsubscribe() - }) - return trackViews(location, lifeCycle, domMutationObservable) + .withConfiguration({ trackViewsManually: true }) + .beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest }) }) @@ -136,246 +296,377 @@ describe('rum track renew session', () => { setupBuilder.cleanup() }) - it('should create new view on renew session', () => { - const { lifeCycle } = setupBuilder.build() - const createSpy = jasmine.createSpy<(event: ViewCreatedEvent) => void>('create') - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy) + describe('location changes', () => { + it('should update view location on search change', () => { + setupBuilder.build() + const { getViewCreateCount, getViewCreate, getViewUpdate, getViewUpdateCount } = viewTest + + history.pushState({}, '', '/foo?bar=qux') + + expect(getViewCreateCount()).toBe(1) + expect(getViewCreate(0).location.href).toMatch(/\/foo$/) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.location.href).toMatch(/\/foo\?bar=qux$/) + expect(lastUpdate.id).toBe(getViewCreate(0).id) + }) - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + it('should update view location on path change', () => { + setupBuilder.build() + const { getViewCreateCount, getViewCreate, getViewUpdate, getViewUpdateCount } = viewTest - expect(createSpy).toHaveBeenCalled() - const viewContext = createSpy.calls.argsFor(0)[0] - expect(viewContext.id).not.toEqual(initialViewId) + history.pushState({}, '', '/bar') + + expect(getViewCreateCount()).toBe(1) + expect(getViewCreate(0).location.href).toMatch(/\/foo$/) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.location.href).toMatch(/\/bar$/) + expect(lastUpdate.id).toBe(getViewCreate(0).id) + }) }) - it('should send a final view event when the session is renewed', () => { - const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) + describe('renew session', () => { + it('should not update the current view when the session is renewed', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + expect(getViewUpdateCount()).toEqual(1) + }) + + it('should not create new view when the session is renewed', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewCreateCount } = viewTest + + expect(getViewCreateCount()).toEqual(1) - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(0).id).not.toBe(getViewEvent(1).id) + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + expect(getViewCreateCount()).toEqual(1) + }) + }) + + describe('view referrer', () => { + it('should set the document referrer as referrer for the initial view', () => { + setupBuilder.build() + const { getViewCreate } = viewTest + + expect(getViewCreate(0).referrer).toEqual(document.referrer) + }) + + it('should set the previous view URL as referrer when starting a new view', () => { + setupBuilder.build() + const { getViewUpdate, getViewUpdateCount, startView } = viewTest + + startView() + history.pushState({}, '', '/bar') + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.referrer).toEqual(jasmine.stringMatching(/\/foo$/)) + expect(lastUpdate.location.href).toEqual(jasmine.stringMatching(/\/bar$/)) + }) + + it('should use the most up-to-date URL of the previous view as a referrer', () => { + setupBuilder.build() + const { getViewCreate, startView } = viewTest + + history.pushState({}, '', '/foo?a=b') + history.pushState({}, '', '/bar') + startView() + + expect(getViewCreate(1).referrer).toEqual(jasmine.stringMatching(/\/bar$/)) + }) }) }) -describe('rum track loading type', () => { +describe('initial view', () => { let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getViewEvent: (index: number) => ViewEvent + let viewTest: ViewTest beforeEach(() => { - ;({ handler, getViewEvent } = spyOnViews()) - - setupBuilder = setup() - .withFakeClock() - .withFakeLocation('/foo') - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - return trackViews(location, lifeCycle, domMutationObservable) - }) + setupBuilder = setup().beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext, 'initial view name') + return viewTest + }) }) afterEach(() => { setupBuilder.cleanup() }) - it('should collect initial view type as "initial_load"', () => { + it('should be created on start', () => { setupBuilder.build() - expect(getViewEvent(0).loadingType).toEqual(ViewLoadingType.INITIAL_LOAD) + const { getViewCreate, getViewCreateCount } = viewTest + + expect(getViewCreateCount()).toBe(1) + expect(getViewCreate(0).name).toBe('initial view name') }) - it('should collect view type as "route_change" after a route change', () => { - setupBuilder.build() - history.pushState({}, '', '/bar') - expect(getViewEvent(1).location.pathname).toEqual('/foo') - expect(getViewEvent(1).loadingType).toEqual(ViewLoadingType.INITIAL_LOAD) + describe('timings', () => { + it('should update timings when notified with a PERFORMANCE_ENTRY_COLLECTED event (throttled)', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdateCount, getViewUpdate } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).timings).toEqual({}) + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) + + expect(getViewUpdateCount()).toEqual(1) - expect(getViewEvent(2).location.pathname).toEqual('/bar') - expect(getViewEvent(2).loadingType).toEqual(ViewLoadingType.ROUTE_CHANGE) + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).timings).toEqual({ + domComplete: 456 as Duration, + domContentLoaded: 345 as Duration, + domInteractive: 234 as Duration, + loadEvent: 567 as Duration, + }) + }) + + it('should update timings when ending a view', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdateCount, getViewUpdate, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + expect(getViewUpdate(0).timings).toEqual({}) + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY) + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY) + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) + expect(getViewUpdateCount()).toEqual(1) + + startView() + + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(1).timings).toEqual({ + domComplete: 456 as Duration, + domContentLoaded: 345 as Duration, + domInteractive: 234 as Duration, + firstContentfulPaint: 123 as Duration, + largestContentfulPaint: 789 as Duration, + loadEvent: 567 as Duration, + }) + expect(getViewUpdate(2).timings).toEqual({}) + }) + + describe('load event happening after initial view end', () => { + let initialView: { init: ViewEvent; end: ViewEvent; last: ViewEvent } + let secondView: { init: ViewEvent; last: ViewEvent } + const VIEW_DURATION = 100 as Duration + + beforeEach(() => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdateCount, getViewUpdate, startView } = viewTest + + expect(getViewUpdateCount()).toEqual(1) + + clock.tick(VIEW_DURATION) + + startView() + + clock.tick(VIEW_DURATION) + + expect(getViewUpdateCount()).toEqual(3) + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY) + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY) + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdateCount()).toEqual(4) + + initialView = { + end: getViewUpdate(1), + init: getViewUpdate(0), + last: getViewUpdate(3), + } + secondView = { + init: getViewUpdate(2), + last: getViewUpdate(2), + } + }) + + it('should not set timings to the second view', () => { + expect(secondView.last.timings).toEqual({}) + }) + + it('should set timings only on the initial view', () => { + expect(initialView.last.timings).toEqual({ + domComplete: 456 as Duration, + domContentLoaded: 345 as Duration, + domInteractive: 234 as Duration, + firstContentfulPaint: 123 as Duration, + largestContentfulPaint: 789 as Duration, + loadEvent: 567 as Duration, + }) + }) + + it('should not update the initial view duration when updating it with new timings', () => { + expect(initialView.end.duration).toBe(VIEW_DURATION) + expect(initialView.last.duration).toBe(VIEW_DURATION) + }) + + it('should update the initial view loadingTime following the loadEventEnd value', () => { + expect(initialView.last.loadingTime).toBe(FAKE_NAVIGATION_ENTRY.loadEventEnd) + }) + }) }) }) -describe('rum track view is active', () => { +describe('view hasReplay', () => { let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getViewEvent: (index: number) => ViewEvent + let viewTest: ViewTest beforeEach(() => { - ;({ handler, getViewEvent } = spyOnViews()) - - setupBuilder = setup() - .withFakeLocation('/foo') - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - return trackViews(location, lifeCycle, domMutationObservable) - }) + setupBuilder = setup().beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest + }) }) afterEach(() => { setupBuilder.cleanup() }) - it('should set initial view as active', () => { + it('sets hasReplay to false by default', () => { setupBuilder.build() - expect(getViewEvent(0).isActive).toBe(true) - }) + const { getViewUpdate } = viewTest - it('should set old view as inactive and new one as active after a route change', () => { - setupBuilder.build() - history.pushState({}, '', '/bar') - expect(getViewEvent(1).isActive).toBe(false) - expect(getViewEvent(2).isActive).toBe(true) + expect(getViewUpdate(0).hasReplay).toBe(false) }) - it('should keep view as active after a search change', () => { - setupBuilder.build() - history.pushState({}, '', '/foo?bar=qux') - expect(getViewEvent(1).isActive).toBe(true) - }) -}) + it('sets hasReplay to true when the recording starts', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdate, startView } = viewTest -describe('rum view timings', () => { - let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getHandledCount: () => number - let getViewEvent: (index: number) => ViewEvent + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) - beforeEach(() => { - ;({ handler, getViewEvent, getHandledCount } = spyOnViews()) + startView() - setupBuilder = setup() - .withFakeLocation('/foo') - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - return trackViews(location, lifeCycle, domMutationObservable) - }) + expect(getViewUpdate(1).hasReplay).toBe(true) }) - afterEach(() => { - setupBuilder.cleanup() - }) + it('keeps hasReplay to true when the recording stops', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdate, startView } = viewTest - it('should update timings when notified with a PERFORMANCE_ENTRY_COLLECTED event (throttled)', () => { - const { lifeCycle, clock } = setupBuilder.withFakeClock().build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).timings).toEqual({}) + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) + startView() - expect(getHandledCount()).toEqual(1) + expect(getViewUpdate(1).hasReplay).toBe(true) + }) - clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + it('sets hasReplay to true when a new view is created after the recording starts', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdate, startView } = viewTest - expect(getHandledCount()).toEqual(2) - expect(getViewEvent(1).timings).toEqual({ - domComplete: 456 as Duration, - domContentLoaded: 345 as Duration, - domInteractive: 234 as Duration, - loadEvent: 567 as Duration, - }) + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + + startView() + + expect(getViewUpdate(2).hasReplay).toBe(true) }) - it('should update timings when ending a view', () => { + it('sets hasReplay to false when a new view is created after the recording stops', () => { const { lifeCycle } = setupBuilder.build() - expect(getHandledCount()).toEqual(1) - expect(getViewEvent(0).timings).toEqual({}) + const { getViewUpdate, startView } = viewTest - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) - expect(getHandledCount()).toEqual(1) + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) - history.pushState({}, '', '/bar') + startView() - expect(getHandledCount()).toEqual(3) - expect(getViewEvent(1).timings).toEqual({ - domComplete: 456 as Duration, - domContentLoaded: 345 as Duration, - domInteractive: 234 as Duration, - firstContentfulPaint: 123 as Duration, - largestContentfulPaint: 789 as Duration, - loadEvent: 567 as Duration, - }) - expect(getViewEvent(2).timings).toEqual({}) + expect(getViewUpdate(2).hasReplay).toBe(false) }) +}) - describe('load event happening after initial view end', () => { - let initialView: { init: ViewEvent; end: ViewEvent; last: ViewEvent } - let secondView: { init: ViewEvent; last: ViewEvent } - const VIEW_DURATION = 100 as Duration +describe('view loading type', () => { + let setupBuilder: TestSetupBuilder + let viewTest: ViewTest - beforeEach(() => { - const { lifeCycle, clock } = setupBuilder.withFakeClock().build() - expect(getHandledCount()).toEqual(1) + beforeEach(() => { + setupBuilder = setup() + .withFakeClock() + .beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest + }) + }) - clock.tick(VIEW_DURATION) + afterEach(() => { + setupBuilder.cleanup() + }) - history.pushState({}, '', '/bar') + it('should collect initial view type as "initial_load"', () => { + setupBuilder.build() + const { getViewUpdate } = viewTest - clock.tick(VIEW_DURATION) + expect(getViewUpdate(0).loadingType).toEqual(ViewLoadingType.INITIAL_LOAD) + }) - expect(getHandledCount()).toEqual(3) + it('should collect view type as "route_change" after a view change', () => { + setupBuilder.build() + const { getViewUpdate, startView } = viewTest - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY) + startView() - clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + expect(getViewUpdate(1).loadingType).toEqual(ViewLoadingType.INITIAL_LOAD) + expect(getViewUpdate(2).loadingType).toEqual(ViewLoadingType.ROUTE_CHANGE) + }) +}) - expect(getHandledCount()).toEqual(4) +describe('view is active', () => { + let setupBuilder: TestSetupBuilder + let viewTest: ViewTest - initialView = { - end: getViewEvent(1), - init: getViewEvent(0), - last: getViewEvent(3), - } - secondView = { - init: getViewEvent(2), - last: getViewEvent(2), - } + beforeEach(() => { + setupBuilder = setup().beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest }) + }) - it('should not set timings to the second view', () => { - expect(secondView.last.timings).toEqual({}) - }) + afterEach(() => { + setupBuilder.cleanup() + }) - it('should set timings only on the initial view', () => { - expect(initialView.last.timings).toEqual({ - domComplete: 456 as Duration, - domContentLoaded: 345 as Duration, - domInteractive: 234 as Duration, - firstContentfulPaint: 123 as Duration, - largestContentfulPaint: 789 as Duration, - loadEvent: 567 as Duration, - }) - }) + it('should set initial view as active', () => { + setupBuilder.build() + const { getViewUpdate } = viewTest - it('should not update the initial view duration when updating it with new timings', () => { - expect(initialView.end.duration).toBe(VIEW_DURATION) - expect(initialView.last.duration).toBe(VIEW_DURATION) - }) + expect(getViewUpdate(0).isActive).toBe(true) + }) - it('should update the initial view loadingTime following the loadEventEnd value', () => { - expect(initialView.last.loadingTime).toBe(FAKE_NAVIGATION_ENTRY.loadEventEnd) - }) + it('should set old view as inactive and new one as active after a route change', () => { + setupBuilder.build() + const { getViewUpdate, startView } = viewTest + + startView() + + expect(getViewUpdate(1).isActive).toBe(false) + expect(getViewUpdate(2).isActive).toBe(true) }) }) -describe('rum track custom timings', () => { +describe('view custom timings', () => { let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getViewEvent: (index: number) => ViewEvent - let addTiming: (name: string, time?: TimeStamp) => void + let viewTest: ViewTest beforeEach(() => { - ;({ handler, getViewEvent } = spyOnViews()) - setupBuilder = setup() - .withFakeLocation('/foo') .withFakeClock() - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - ;({ addTiming } = trackViews(location, lifeCycle, domMutationObservable)) + .beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest }) }) @@ -385,26 +676,30 @@ describe('rum track custom timings', () => { it('should add custom timing to current view', () => { const { clock } = setupBuilder.build() - history.pushState({}, '', '/bar') - const currentViewId = getViewEvent(2).id + const { getViewUpdate, startView, addTiming } = viewTest + + startView() + const currentViewId = getViewUpdate(2).id clock.tick(20) addTiming('foo') - const event = getViewEvent(3) - expect(event.id).toEqual(currentViewId) - expect(event.customTimings).toEqual({ foo: 20 as Duration }) + const view = getViewUpdate(3) + expect(view.id).toEqual(currentViewId) + expect(view.customTimings).toEqual({ foo: 20 as Duration }) }) it('should add multiple custom timings', () => { const { clock } = setupBuilder.build() + const { getViewUpdate, addTiming } = viewTest + clock.tick(20) addTiming('foo') clock.tick(10) addTiming('bar') - const event = getViewEvent(2) - expect(event.customTimings).toEqual({ + const view = getViewUpdate(2) + expect(view.customTimings).toEqual({ bar: 30 as Duration, foo: 20 as Duration, }) @@ -412,14 +707,16 @@ describe('rum track custom timings', () => { it('should update custom timing', () => { const { clock } = setupBuilder.build() + const { getViewUpdate, addTiming } = viewTest + clock.tick(20) addTiming('foo') clock.tick(10) addTiming('bar') - let event = getViewEvent(2) - expect(event.customTimings).toEqual({ + let view = getViewUpdate(2) + expect(view.customTimings).toEqual({ bar: 30 as Duration, foo: 20 as Duration, }) @@ -427,8 +724,8 @@ describe('rum track custom timings', () => { clock.tick(20) addTiming('foo') - event = getViewEvent(3) - expect(event.customTimings).toEqual({ + view = getViewUpdate(3) + expect(view.customTimings).toEqual({ bar: 30 as Duration, foo: 50 as Duration, }) @@ -436,111 +733,40 @@ describe('rum track custom timings', () => { it('should add custom timing with a specific time', () => { const { clock } = setupBuilder.build() + const { getViewUpdate, addTiming } = viewTest clock.tick(1234) addTiming('foo', timeStampNow()) - expect(getViewEvent(1).customTimings).toEqual({ + expect(getViewUpdate(1).customTimings).toEqual({ foo: 1234 as Duration, }) }) it('should sanitized timing name', () => { const { clock } = setupBuilder.build() + const { getViewUpdate, addTiming } = viewTest + const displaySpy = spyOn(display, 'warn') clock.tick(1234) addTiming('foo bar-qux.@zip_21%$*€👋', timeStampNow()) - expect(getViewEvent(1).customTimings).toEqual({ + expect(getViewUpdate(1).customTimings).toEqual({ 'foo_bar-qux.@zip_21_$____': 1234 as Duration, }) expect(displaySpy).toHaveBeenCalled() }) }) -describe('track hasReplay', () => { - let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getViewEvent: (index: number) => ViewEvent - - beforeEach(() => { - ;({ handler, getViewEvent } = spyOnViews()) - - setupBuilder = setup() - .withFakeLocation('/foo') - .withFakeClock() - .beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - return trackViews(location, lifeCycle, domMutationObservable) - }) - }) - - afterEach(() => { - setupBuilder.cleanup() - }) - - it('sets hasReplay to false by default', () => { - setupBuilder.build() - expect(getViewEvent(0).hasReplay).toBe(false) - }) - - it('sets hasReplay to true when the recording starts', () => { - const { lifeCycle } = setupBuilder.build() - - lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) - - history.pushState({}, '', '/bar') - - expect(getViewEvent(1).hasReplay).toBe(true) - }) - - it('keeps hasReplay to true when the recording stops', () => { - const { lifeCycle } = setupBuilder.build() - - lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) - lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) - - history.pushState({}, '', '/bar') - - expect(getViewEvent(1).hasReplay).toBe(true) - }) - - it('sets hasReplay to true when a new view is created after the recording starts', () => { - const { lifeCycle } = setupBuilder.build() - - lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) - - history.pushState({}, '', '/bar') - - expect(getViewEvent(2).hasReplay).toBe(true) - }) - - it('sets hasReplay to false when a new view is created after the recording stops', () => { - const { lifeCycle } = setupBuilder.build() - - lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) - lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) - - history.pushState({}, '', '/bar') - - expect(getViewEvent(2).hasReplay).toBe(false) - }) -}) - -describe('rum start view', () => { +describe('start view', () => { let setupBuilder: TestSetupBuilder - let handler: jasmine.Spy - let getViewEvent: (index: number) => ViewEvent - let getHandledCount: () => number - let startView: (name?: string, endClocks?: ClocksState) => void + let viewTest: ViewTest beforeEach(() => { - ;({ getHandledCount, getViewEvent, handler } = spyOnViews()) - - setupBuilder = setup().beforeBuild(({ location, lifeCycle, domMutationObservable }) => { - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) - ;({ startView } = trackViews(location, lifeCycle, domMutationObservable)) + setupBuilder = setup().beforeBuild((buildContext) => { + viewTest = setupViewTest(buildContext) + return viewTest }) }) @@ -550,43 +776,47 @@ describe('rum start view', () => { it('should start a new view', () => { const { clock } = setupBuilder.withFakeClock().build() - expect(getHandledCount()).toBe(1) - const initialViewId = getViewEvent(0).id + const { getViewUpdateCount, getViewUpdate, startView } = viewTest + + expect(getViewUpdateCount()).toBe(1) + const initialViewId = getViewUpdate(0).id clock.tick(10) startView() - expect(getHandledCount()).toBe(3) + expect(getViewUpdateCount()).toBe(3) - expect(getViewEvent(1).id).toBe(initialViewId) - expect(getViewEvent(1).isActive).toBe(false) - expect(getViewEvent(1).startClocks.relative).toBe(0 as RelativeTime) - expect(getViewEvent(1).duration).toBe(10 as Duration) + expect(getViewUpdate(1).id).toBe(initialViewId) + expect(getViewUpdate(1).isActive).toBe(false) + expect(getViewUpdate(1).startClocks.relative).toBe(0 as RelativeTime) + expect(getViewUpdate(1).duration).toBe(10 as Duration) - expect(getViewEvent(2).id).not.toBe(initialViewId) - expect(getViewEvent(2).isActive).toBe(true) - expect(getViewEvent(2).startClocks.relative).toBe(10 as RelativeTime) + expect(getViewUpdate(2).id).not.toBe(initialViewId) + expect(getViewUpdate(2).isActive).toBe(true) + expect(getViewUpdate(2).startClocks.relative).toBe(10 as RelativeTime) }) it('should name the view', () => { setupBuilder.build() + const { getViewUpdate, startView } = viewTest startView() startView('foo') startView('bar') - expect(getViewEvent(2).name).toBeUndefined() - expect(getViewEvent(4).name).toBe('foo') - expect(getViewEvent(6).name).toBe('bar') + expect(getViewUpdate(2).name).toBeUndefined() + expect(getViewUpdate(4).name).toBe('foo') + expect(getViewUpdate(6).name).toBe('bar') }) it('should use the provided clock to stop the current view and start the new one', () => { const { clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, startView } = viewTest clock.tick(100) startView('foo', relativeToClocks(50 as RelativeTime)) - expect(getViewEvent(1).duration).toBe(50 as Duration) - expect(getViewEvent(2).startClocks.relative).toBe(50 as RelativeTime) + expect(getViewUpdate(1).duration).toBe(50 as Duration) + expect(getViewUpdate(2).startClocks.relative).toBe(50 as RelativeTime) }) }) diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index f4c21b44e9..5d71c247fb 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -31,7 +31,7 @@ export interface TestSetupBuilder { } type BeforeBuildCallback = (buildContext: BuildContext) => void | { stop: () => void } -interface BuildContext { +export interface BuildContext { lifeCycle: LifeCycle domMutationObservable: Observable configuration: Readonly From a4bb9ae8272b0a21838868c0d780c9ed7ce6d312 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 2 Jun 2021 16:49:12 +0200 Subject: [PATCH 5/8] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20rename=20boot/[pack?= =?UTF-8?q?age].ts=20to=20boot/start[package].ts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/logs/src/boot/logs.entry.ts | 2 +- packages/logs/src/boot/{logs.spec.ts => startLogs.spec.ts} | 2 +- packages/logs/src/boot/{logs.ts => startLogs.ts} | 0 packages/logs/src/index.ts | 2 +- packages/rum-core/src/boot/rumPublicApi.ts | 2 +- packages/rum-core/src/boot/{rum.spec.ts => startRum.spec.ts} | 2 +- packages/rum-core/src/boot/{rum.ts => startRum.ts} | 0 packages/rum-core/src/index.ts | 2 +- packages/rum-recorder/src/boot/recorder.entry.ts | 2 +- packages/rum-recorder/src/boot/rumRecorderPublicApi.ts | 2 +- .../src/boot/{recorder.spec.ts => startRecording.spec.ts} | 2 +- .../rum-recorder/src/boot/{recorder.ts => startRecording.ts} | 0 12 files changed, 9 insertions(+), 9 deletions(-) rename packages/logs/src/boot/{logs.spec.ts => startLogs.spec.ts} (99%) rename packages/logs/src/boot/{logs.ts => startLogs.ts} (100%) rename packages/rum-core/src/boot/{rum.spec.ts => startRum.spec.ts} (99%) rename packages/rum-core/src/boot/{rum.ts => startRum.ts} (100%) rename packages/rum-recorder/src/boot/{recorder.spec.ts => startRecording.spec.ts} (99%) rename packages/rum-recorder/src/boot/{recorder.ts => startRecording.ts} (100%) diff --git a/packages/logs/src/boot/logs.entry.ts b/packages/logs/src/boot/logs.entry.ts index 26ee2f269a..9ac3bf9fed 100644 --- a/packages/logs/src/boot/logs.entry.ts +++ b/packages/logs/src/boot/logs.entry.ts @@ -11,7 +11,7 @@ import { display, } from '@datadog/browser-core' import { HandlerType, Logger, LogsMessage, StatusType } from '../domain/logger' -import { startLogs, LogsUserConfiguration } from './logs' +import { startLogs, LogsUserConfiguration } from './startLogs' export interface LoggerConfiguration { level?: StatusType diff --git a/packages/logs/src/boot/logs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts similarity index 99% rename from packages/logs/src/boot/logs.spec.ts rename to packages/logs/src/boot/startLogs.spec.ts index 321669a86f..13ab853b2b 100644 --- a/packages/logs/src/boot/logs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -16,7 +16,7 @@ import { Clock, mockClock } from '../../../core/test/specHelper' import { Logger, LogsMessage, StatusType } from '../domain/logger' import { LogsEvent } from '../logsEvent.types' -import { buildAssemble, doStartLogs } from './logs' +import { buildAssemble, doStartLogs } from './startLogs' interface SentMessage extends LogsMessage { logger?: { name: string } diff --git a/packages/logs/src/boot/logs.ts b/packages/logs/src/boot/startLogs.ts similarity index 100% rename from packages/logs/src/boot/logs.ts rename to packages/logs/src/boot/startLogs.ts diff --git a/packages/logs/src/index.ts b/packages/logs/src/index.ts index b653e46c07..081f3a1da3 100644 --- a/packages/logs/src/index.ts +++ b/packages/logs/src/index.ts @@ -1,4 +1,4 @@ export { Logger, LogsMessage, StatusType, HandlerType } from './domain/logger' export { LoggerConfiguration, LogsPublicApi as LogsGlobal, datadogLogs } from './boot/logs.entry' -export { LogsUserConfiguration } from './boot/logs' +export { LogsUserConfiguration } from './boot/startLogs' export { LogsEvent } from './logsEvent.types' diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 92d2e2fe51..c2f6e42100 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -22,7 +22,7 @@ import { ProvidedSource } from '../domain/rumEventsCollection/error/errorCollect import { CommonContext, User, ActionType } from '../rawRumEvent.types' import { RumEvent } from '../rumEvent.types' import { buildEnv } from './buildEnv' -import { startRum } from './rum' +import { startRum } from './startRum' export interface RumUserConfiguration extends UserConfiguration { applicationId: string diff --git a/packages/rum-core/src/boot/rum.spec.ts b/packages/rum-core/src/boot/startRum.spec.ts similarity index 99% rename from packages/rum-core/src/boot/rum.spec.ts rename to packages/rum-core/src/boot/startRum.spec.ts index f4ad900254..ec6e93c434 100644 --- a/packages/rum-core/src/boot/rum.spec.ts +++ b/packages/rum-core/src/boot/startRum.spec.ts @@ -9,7 +9,7 @@ import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle' import { SESSION_KEEP_ALIVE_INTERVAL, THROTTLE_VIEW_UPDATE_PERIOD } from '../domain/rumEventsCollection/view/trackViews' import { startViewCollection } from '../domain/rumEventsCollection/view/viewCollection' import { RumEvent } from '../rumEvent.types' -import { startRumEventCollection } from './rum' +import { startRumEventCollection } from './startRum' function collectServerEvents(lifeCycle: LifeCycle) { const serverRumEvents: RumEvent[] = [] diff --git a/packages/rum-core/src/boot/rum.ts b/packages/rum-core/src/boot/startRum.ts similarity index 100% rename from packages/rum-core/src/boot/rum.ts rename to packages/rum-core/src/boot/startRum.ts diff --git a/packages/rum-core/src/index.ts b/packages/rum-core/src/index.ts index 1f7412e06d..2a2fd62aa0 100644 --- a/packages/rum-core/src/index.ts +++ b/packages/rum-core/src/index.ts @@ -10,7 +10,7 @@ export { RumLongTaskEvent, } from './rumEvent.types' export { ViewContext, CommonContext } from './rawRumEvent.types' -export { startRum } from './boot/rum' +export { startRum } from './boot/startRum' export { LifeCycle, LifeCycleEventType } from './domain/lifeCycle' export { ParentContexts } from './domain/parentContexts' export { RumSession } from './domain/rumSession' diff --git a/packages/rum-recorder/src/boot/recorder.entry.ts b/packages/rum-recorder/src/boot/recorder.entry.ts index 860815feaf..08c02facc5 100644 --- a/packages/rum-recorder/src/boot/recorder.entry.ts +++ b/packages/rum-recorder/src/boot/recorder.entry.ts @@ -1,7 +1,7 @@ import { defineGlobal, getGlobalObject } from '@datadog/browser-core' import { startRum } from '@datadog/browser-rum-core' -import { startRecording } from './recorder' +import { startRecording } from './startRecording' import { RumRecorderPublicApi, makeRumRecorderPublicApi } from './rumRecorderPublicApi' export const datadogRum = makeRumRecorderPublicApi(startRum, startRecording) diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts index 8897bab9d3..01935d5897 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts @@ -7,7 +7,7 @@ import { CommonContext, } from '@datadog/browser-rum-core' -import { startRecording } from './recorder' +import { startRecording } from './startRecording' export type StartRecording = typeof startRecording export type RumRecorderPublicApi = ReturnType diff --git a/packages/rum-recorder/src/boot/recorder.spec.ts b/packages/rum-recorder/src/boot/startRecording.spec.ts similarity index 99% rename from packages/rum-recorder/src/boot/recorder.spec.ts rename to packages/rum-recorder/src/boot/startRecording.spec.ts index 8d7b6c3e86..a0cedbbdfe 100644 --- a/packages/rum-recorder/src/boot/recorder.spec.ts +++ b/packages/rum-recorder/src/boot/startRecording.spec.ts @@ -8,7 +8,7 @@ import { collectAsyncCalls } from '../../test/utils' import { setMaxSegmentSize } from '../domain/segmentCollection/segmentCollection' import { Segment, RecordType } from '../types' -import { startRecording } from './recorder' +import { startRecording } from './startRecording' describe('startRecording', () => { let setupBuilder: TestSetupBuilder diff --git a/packages/rum-recorder/src/boot/recorder.ts b/packages/rum-recorder/src/boot/startRecording.ts similarity index 100% rename from packages/rum-recorder/src/boot/recorder.ts rename to packages/rum-recorder/src/boot/startRecording.ts From d9547e1ab55ef99a51f131e23d051723e9dc0974 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Fri, 4 Jun 2021 16:29:42 +0200 Subject: [PATCH 6/8] add eslint rule to check spec import inside spec --- .eslintrc.js | 6 ++++ eslint-local-rules/disallowSpecImportSpec.js | 36 ++++++++++++++++++++ eslint-local-rules/index.js | 1 + 3 files changed, 43 insertions(+) create mode 100644 eslint-local-rules/disallowSpecImportSpec.js diff --git a/.eslintrc.js b/.eslintrc.js index 75de803d48..011f8a1fe4 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -245,6 +245,12 @@ module.exports = { 'local-rules/disallow-side-effects': 'error', }, }, + { + files: ['packages/*/src/**/*.spec.ts'], + rules: { + 'local-rules/disallow-spec-import-spec': 'error', + }, + }, { files: ['packages/{rum,logs,rum-recorder}/src/index.ts', 'packages/rum-recorder/src/internal.ts'], rules: { diff --git a/eslint-local-rules/disallowSpecImportSpec.js b/eslint-local-rules/disallowSpecImportSpec.js new file mode 100644 index 0000000000..43a7157123 --- /dev/null +++ b/eslint-local-rules/disallowSpecImportSpec.js @@ -0,0 +1,36 @@ +module.exports = { + meta: { + docs: { + description: + // eslint-disable-next-line max-len + 'Disallow importing spec file code inside a spec file, it would execute the tests from the imported spec file twice', + recommended: false, + }, + schema: [], + }, + create(context) { + if (!isSpecFile(context.getFilename())) { + return {} + } + return { + Program(node) { + reportSpecImportSpec(context, node) + }, + } + }, +} + +function isSpecFile(filename) { + return /\.spec(\.ts)?$/.test(filename) +} + +function reportSpecImportSpec(context, node) { + if (node.type === 'Program') { + node.body.forEach((child) => reportSpecImportSpec(context, child)) + } else if (node.type === 'ImportDeclaration' && isSpecFile(node.source.value)) { + context.report({ + node: node.source, + message: 'A spec file must not import members of another spec file', + }) + } +} diff --git a/eslint-local-rules/index.js b/eslint-local-rules/index.js index 4294c1762e..b74cfa5e7c 100644 --- a/eslint-local-rules/index.js +++ b/eslint-local-rules/index.js @@ -9,4 +9,5 @@ module.exports = { 'disallow-side-effects': require('./disallowSideEffects'), 'disallow-enum-exports': require('./disallowEnumExports'), + 'disallow-spec-import-spec': require('./disallowSpecImportSpec'), } From 4fcff0d03b724b6573fec9ff087ae089740a8675 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Fri, 4 Jun 2021 16:35:03 +0200 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=90=9B=20avoid=20to=20execute=20view?= =?UTF-8?q?=20tests=20twice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViewMetrics.spec.ts | 3 +- .../view/trackViews.spec.ts | 50 +------------------ packages/rum-core/test/specHelper.ts | 49 +++++++++++++++++- 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 054f3b2390..fe90c46625 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -1,6 +1,6 @@ import { Context, RelativeTime, Duration } from '@datadog/browser-core' import { LifeCycleEventType, RumEvent } from '@datadog/browser-rum-core' -import { TestSetupBuilder, setup } from '../../../../test/specHelper' +import { TestSetupBuilder, setup, setupViewTest, ViewTest } from '../../../../test/specHelper' import { RumPerformanceNavigationTiming } from '../../../browser/performanceCollection' import { RumEventType } from '../../../rawRumEvent.types' import { @@ -9,7 +9,6 @@ import { PAGE_ACTIVITY_VALIDATION_DELAY, } from '../../trackPageActivities' import { THROTTLE_VIEW_UPDATE_PERIOD } from './trackViews' -import { setupViewTest, ViewTest } from './trackViews.spec' const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = (PAGE_ACTIVITY_VALIDATION_DELAY * 0.8) as Duration diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts index 3db351c2d6..6157477648 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -1,5 +1,5 @@ import { Duration, RelativeTime, timeStampNow, display, relativeToClocks } from '@datadog/browser-core' -import { setup, TestSetupBuilder, BuildContext } from '../../../../test/specHelper' +import { setup, TestSetupBuilder, setupViewTest, ViewTest } from '../../../../test/specHelper' import { RumLargestContentfulPaintTiming, RumPerformanceNavigationTiming, @@ -7,7 +7,7 @@ import { } from '../../../browser/performanceCollection' import { ViewLoadingType } from '../../../rawRumEvent.types' import { LifeCycleEventType } from '../../lifeCycle' -import { THROTTLE_VIEW_UPDATE_PERIOD, trackViews, ViewEvent } from './trackViews' +import { THROTTLE_VIEW_UPDATE_PERIOD, ViewEvent } from './trackViews' const FAKE_PAINT_ENTRY: RumPerformancePaintTiming = { entryType: 'paint', @@ -26,52 +26,6 @@ const FAKE_NAVIGATION_ENTRY: RumPerformanceNavigationTiming = { loadEventEnd: 567 as RelativeTime, } -export type ViewTest = ReturnType - -export function setupViewTest( - { lifeCycle, location, domMutationObservable, configuration }: BuildContext, - initialViewName?: string -) { - const { handler: viewUpdateHandler, getViewEvent: getViewUpdate, getHandledCount: getViewUpdateCount } = spyOnViews( - 'view update' - ) - lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, viewUpdateHandler) - const { handler: viewCreateHandler, getViewEvent: getViewCreate, getHandledCount: getViewCreateCount } = spyOnViews( - 'view create' - ) - lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, viewCreateHandler) - const { stop, startView, addTiming } = trackViews( - location, - lifeCycle, - domMutationObservable, - !configuration.trackViewsManually, - initialViewName - ) - return { - stop, - startView, - addTiming, - getViewUpdate, - getViewUpdateCount, - getViewCreate, - getViewCreateCount, - } -} - -function spyOnViews(name?: string) { - const handler = jasmine.createSpy(name) - - function getViewEvent(index: number) { - return handler.calls.argsFor(index)[0] as ViewEvent - } - - function getHandledCount() { - return handler.calls.count() - } - - return { handler, getViewEvent, getHandledCount } -} - describe('track views automatically', () => { let setupBuilder: TestSetupBuilder let viewTest: ViewTest diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index 5d71c247fb..360a972d35 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -10,9 +10,10 @@ import { noop, } from '@datadog/browser-core' import { SPEC_ENDPOINTS, mockClock, Clock } from '../../core/test/specHelper' +import { ForegroundContexts } from '../src/domain/foregroundContexts' import { LifeCycle, LifeCycleEventType } from '../src/domain/lifeCycle' import { ParentContexts } from '../src/domain/parentContexts' -import { ForegroundContexts } from '../src/domain/foregroundContexts' +import { trackViews, ViewEvent } from '../src/domain/rumEventsCollection/view/trackViews' import { RumSession } from '../src/domain/rumSession' import { CommonContext, RawRumEvent, RumContext, ViewContext } from '../src/rawRumEvent.types' import { validateFormat } from './formatValidation' @@ -206,3 +207,49 @@ function validateRumEventFormat(rawRumEvent: RawRumEvent) { } validateFormat(combine(fakeContext, rawRumEvent)) } + +export type ViewTest = ReturnType + +export function setupViewTest( + { lifeCycle, location, domMutationObservable, configuration }: BuildContext, + initialViewName?: string +) { + const { handler: viewUpdateHandler, getViewEvent: getViewUpdate, getHandledCount: getViewUpdateCount } = spyOnViews( + 'view update' + ) + lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, viewUpdateHandler) + const { handler: viewCreateHandler, getViewEvent: getViewCreate, getHandledCount: getViewCreateCount } = spyOnViews( + 'view create' + ) + lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, viewCreateHandler) + const { stop, startView, addTiming } = trackViews( + location, + lifeCycle, + domMutationObservable, + !configuration.trackViewsManually, + initialViewName + ) + return { + stop, + startView, + addTiming, + getViewUpdate, + getViewUpdateCount, + getViewCreate, + getViewCreateCount, + } +} + +export function spyOnViews(name?: string) { + const handler = jasmine.createSpy(name) + + function getViewEvent(index: number) { + return handler.calls.argsFor(index)[0] as ViewEvent + } + + function getHandledCount() { + return handler.calls.count() + } + + return { handler, getViewEvent, getHandledCount } +} From 5be4f1bf3c6b5e65deeff090d8e30949cb66b822 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Tue, 8 Jun 2021 11:46:51 +0200 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=91=8C=20check=20spec=20import=20ever?= =?UTF-8?q?ywhere=20+=20simplification?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .eslintrc.js | 4 +-- eslint-local-rules/disallowSpecImport.js | 27 +++++++++++++++ eslint-local-rules/disallowSpecImportSpec.js | 36 -------------------- eslint-local-rules/index.js | 2 +- 4 files changed, 30 insertions(+), 39 deletions(-) create mode 100644 eslint-local-rules/disallowSpecImport.js delete mode 100644 eslint-local-rules/disallowSpecImportSpec.js diff --git a/.eslintrc.js b/.eslintrc.js index 011f8a1fe4..b1e8b61213 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -246,9 +246,9 @@ module.exports = { }, }, { - files: ['packages/*/src/**/*.spec.ts'], + files: ['packages/**/*.ts'], rules: { - 'local-rules/disallow-spec-import-spec': 'error', + 'local-rules/disallow-spec-import': 'error', }, }, { diff --git a/eslint-local-rules/disallowSpecImport.js b/eslint-local-rules/disallowSpecImport.js new file mode 100644 index 0000000000..0b04f0d8e4 --- /dev/null +++ b/eslint-local-rules/disallowSpecImport.js @@ -0,0 +1,27 @@ +module.exports = { + meta: { + docs: { + description: + // eslint-disable-next-line max-len + 'Disallow importing spec file code to avoid to execute the tests from the imported spec file twice', + recommended: false, + }, + schema: [], + }, + create(context) { + return { + ImportDeclaration(node) { + if (isSpecFile(node.source.value)) { + context.report({ + node: node.source, + message: 'Members of a spec file must not be imported', + }) + } + }, + } + }, +} + +function isSpecFile(filename) { + return /\.spec(\.ts)?$/.test(filename) +} diff --git a/eslint-local-rules/disallowSpecImportSpec.js b/eslint-local-rules/disallowSpecImportSpec.js deleted file mode 100644 index 43a7157123..0000000000 --- a/eslint-local-rules/disallowSpecImportSpec.js +++ /dev/null @@ -1,36 +0,0 @@ -module.exports = { - meta: { - docs: { - description: - // eslint-disable-next-line max-len - 'Disallow importing spec file code inside a spec file, it would execute the tests from the imported spec file twice', - recommended: false, - }, - schema: [], - }, - create(context) { - if (!isSpecFile(context.getFilename())) { - return {} - } - return { - Program(node) { - reportSpecImportSpec(context, node) - }, - } - }, -} - -function isSpecFile(filename) { - return /\.spec(\.ts)?$/.test(filename) -} - -function reportSpecImportSpec(context, node) { - if (node.type === 'Program') { - node.body.forEach((child) => reportSpecImportSpec(context, child)) - } else if (node.type === 'ImportDeclaration' && isSpecFile(node.source.value)) { - context.report({ - node: node.source, - message: 'A spec file must not import members of another spec file', - }) - } -} diff --git a/eslint-local-rules/index.js b/eslint-local-rules/index.js index b74cfa5e7c..d17409e8cf 100644 --- a/eslint-local-rules/index.js +++ b/eslint-local-rules/index.js @@ -9,5 +9,5 @@ module.exports = { 'disallow-side-effects': require('./disallowSideEffects'), 'disallow-enum-exports': require('./disallowEnumExports'), - 'disallow-spec-import-spec': require('./disallowSpecImportSpec'), + 'disallow-spec-import': require('./disallowSpecImport'), }