From 3942df858b80ffe7faf0041bdb2e91c38c43218d Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Fri, 30 Oct 2020 17:12:07 +0100 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=90=9B=20use=20the=20correct=20schema?= =?UTF-8?q?=20to=20validate=20events?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/test/formatValidation.ts | 7 +++---- test/unit/karma.base.conf.js | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/rum/test/formatValidation.ts b/packages/rum/test/formatValidation.ts index 9072924b03..50bb6d7f93 100644 --- a/packages/rum/test/formatValidation.ts +++ b/packages/rum/test/formatValidation.ts @@ -12,7 +12,7 @@ export function validateFormat(rumEvent: Context) { const instance = new ajv({ allErrors: true, }) - const valid = instance + instance .addSchema(_commonSchemaJson, 'schemas/_common-schema.json') .addSchema(viewSchemaJson, 'schemas/view-schema.json') .addSchema(actionSchemaJson, 'schemas/action-schema.json') @@ -20,10 +20,9 @@ export function validateFormat(rumEvent: Context) { .addSchema(long_taskSchemaJson, 'schemas/long_task-schema.json') .addSchema(errorSchemaJson, 'schemas/error-schema.json') .addSchema(rumEventsFormatJson, 'rum-events-format.json') - .validate('schemas/_common-schema.json', rumEvent) + .validate('rum-events-format.json', rumEvent) - expect(valid).toBe(true, 'invalid rum event') if (instance.errors) { - instance.errors.map((error) => expect(error.message).toBeUndefined()) + instance.errors.map((error) => fail(`${error.dataPath || 'event'} ${error.message}`)) } } diff --git a/test/unit/karma.base.conf.js b/test/unit/karma.base.conf.js index 9fa213b07a..3c613067a6 100644 --- a/test/unit/karma.base.conf.js +++ b/test/unit/karma.base.conf.js @@ -15,7 +15,7 @@ module.exports = { client: { jasmine: { random: true, - oneFailurePerSpec: true, + oneFailurePerSpec: false, }, }, preprocessors: { From 6cec8f1e28fce477f22f9e3ff411bba456c7b0f3 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Fri, 30 Oct 2020 18:02:21 +0100 Subject: [PATCH 2/5] make rum-events-format track master --- .gitmodules | 1 + package.json | 2 +- rum-events-format | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index 3a8aa5a842..56d90933b5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,4 @@ [submodule "rum-events-format"] path = rum-events-format url = https://github.com/DataDog/rum-events-format + branch = master diff --git a/package.json b/package.json index 058012d9a5..12e9e1bc29 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "packages/*" ], "scripts": { - "postinstall": "git submodule update --init", + "postinstall": "git submodule update --init && git submodule update --remote", "build": "lerna run build --stream", "build:bundle": "lerna run build:bundle --stream", "format": "prettier --check \"**/*.{ts,js,json,md,yml,html}\"", diff --git a/rum-events-format b/rum-events-format index 9315200ab7..0ae8f253c6 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit 9315200ab7ed896b42710065206c1bf5453ac571 +Subproject commit 0ae8f253c60662158a1342d8b680243a14d03578 From fc1f30e72ecfe9476c4667eae7d046410fe42b08 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Fri, 30 Oct 2020 18:43:30 +0100 Subject: [PATCH 3/5] fix unit tests --- packages/rum/src/domain/assemblyV2.spec.ts | 125 ++++++++++++------ .../action/actionCollection.spec.ts | 4 +- .../resource/resourceCollection.spec.ts | 21 ++- packages/rum/test/createRawRumEvent.ts | 78 +++++++++++ 4 files changed, 182 insertions(+), 46 deletions(-) create mode 100644 packages/rum/test/createRawRumEvent.ts diff --git a/packages/rum/src/domain/assemblyV2.spec.ts b/packages/rum/src/domain/assemblyV2.spec.ts index 45bb04d918..01a68e9266 100644 --- a/packages/rum/src/domain/assemblyV2.spec.ts +++ b/packages/rum/src/domain/assemblyV2.spec.ts @@ -1,6 +1,7 @@ import { Context } from '@datadog/browser-core' +import { createRawRumEvent } from '../../test/createRawRumEvent' import { setup, TestSetupBuilder } from '../../test/specHelper' -import { RawRumEventV2, RumEventType } from '../typesV2' +import { RumEventType } from '../typesV2' import { LifeCycle, LifeCycleEventType } from './lifeCycle' interface ServerRumEvents { @@ -37,21 +38,6 @@ describe('rum assembly v2', () => { let isTracked: boolean let viewSessionId: string | undefined - function generateRawRumEvent( - type: RumEventType, - properties?: Partial, - savedGlobalContext?: Context, - customerContext?: Context - ) { - const event = { type, ...properties } - lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { - customerContext, - savedGlobalContext, - rawRumEvent: event as RawRumEventV2, - startTime: 0, - }) - } - beforeEach(() => { isTracked = true viewSessionId = '1234' @@ -93,7 +79,10 @@ describe('rum assembly v2', () => { describe('events', () => { it('should have snake cased attributes', () => { - generateRawRumEvent(RumEventType.LONG_TASK, { longTask: { duration: 2 } }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.LONG_TASK, { longTask: { duration: 2 } }), + startTime: 0, + }) expect(serverRumEvents[0].long_task!.duration).toBe(2) }) @@ -101,20 +90,29 @@ describe('rum assembly v2', () => { describe('rum context', () => { it('should be merged with event attributes', () => { - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW, undefined), + startTime: 0, + }) expect(serverRumEvents[0].view.id).toBeDefined() expect(serverRumEvents[0].date).toBeDefined() }) it('should be snake cased', () => { - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW, undefined), + startTime: 0, + }) expect(serverRumEvents[0]._dd.format_version).toBe(2) }) it('should be overwritten by event attributes', () => { - generateRawRumEvent(RumEventType.VIEW, { date: 10 }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW, { date: 10 }), + startTime: 0, + }) expect(serverRumEvents[0].date).toBe(10) }) @@ -123,7 +121,10 @@ describe('rum assembly v2', () => { describe('rum global context', () => { it('should be merged with event attributes', () => { setGlobalContext({ bar: 'foo' }) - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect((serverRumEvents[0].context as any).bar).toEqual('foo') }) @@ -131,9 +132,15 @@ describe('rum assembly v2', () => { it('should ignore subsequent context mutation', () => { const globalContext = { bar: 'foo' } setGlobalContext(globalContext) - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) delete globalContext.bar - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect((serverRumEvents[0].context as any).bar).toEqual('foo') expect((serverRumEvents[1].context as any).bar).toBeUndefined() @@ -141,7 +148,10 @@ describe('rum assembly v2', () => { it('should not be automatically snake cased', () => { setGlobalContext({ fooBar: 'foo' }) - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect((serverRumEvents[0].context as any).fooBar).toEqual('foo') }) @@ -149,7 +159,11 @@ describe('rum assembly v2', () => { it('should ignore the current global context when a saved global context is provided', () => { setGlobalContext({ replacedContext: 'b', addedContext: 'x' }) - generateRawRumEvent(RumEventType.VIEW, undefined, { replacedContext: 'a' }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + savedGlobalContext: { replacedContext: 'a' }, + startTime: 0, + }) expect((serverRumEvents[0].context as any).replacedContext).toEqual('a') expect((serverRumEvents[0].context as any).addedContext).toEqual(undefined) @@ -158,13 +172,21 @@ describe('rum assembly v2', () => { describe('customer context', () => { it('should be merged with event attributes', () => { - generateRawRumEvent(RumEventType.VIEW, undefined, undefined, { foo: 'bar' }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + customerContext: { foo: 'bar' }, + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect((serverRumEvents[0].context as any).foo).toEqual('bar') }) it('should not be automatically snake cased', () => { - generateRawRumEvent(RumEventType.VIEW, undefined, undefined, { fooBar: 'foo' }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + customerContext: { fooBar: 'foo' }, + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect((serverRumEvents[0].context as any).fooBar).toEqual('foo') }) @@ -173,21 +195,36 @@ describe('rum assembly v2', () => { describe('action context', () => { it('should be added on some event categories', () => { ;[RumEventType.RESOURCE, RumEventType.LONG_TASK, RumEventType.ERROR].forEach((category) => { - generateRawRumEvent(category) - expect(serverRumEvents[0].action.id).toBeDefined() + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(category), + startTime: 0, + }) + expect(serverRumEvents[0].action).toEqual({ id: '7890' }) serverRumEvents = [] }) - ;[RumEventType.VIEW, RumEventType.ACTION].forEach((category) => { - generateRawRumEvent(category) - expect(serverRumEvents[0].action).not.toBeDefined() - serverRumEvents = [] + + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) + expect(serverRumEvents[0].action).not.toBeDefined() + serverRumEvents = [] + + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.ACTION), + startTime: 0, }) + expect(serverRumEvents[0].action.id).not.toBeDefined() + serverRumEvents = [] }) }) describe('view context', () => { it('should be merged with event attributes', () => { - generateRawRumEvent(RumEventType.ACTION) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.ACTION), + startTime: 0, + }) expect(serverRumEvents[0].view).toEqual({ id: 'abcde', referrer: 'url', @@ -201,28 +238,40 @@ describe('rum assembly v2', () => { it('when tracked, it should generate event', () => { isTracked = true - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect(serverRumEvents.length).toBe(1) }) it('when not tracked, it should not generate event', () => { isTracked = false - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect(serverRumEvents.length).toBe(0) }) it('when view context has session id, it should generate event', () => { viewSessionId = '1234' - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect(serverRumEvents.length).toBe(1) }) it('when view context has no session id, it should not generate event', () => { viewSessionId = undefined - generateRawRumEvent(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0, + }) expect(serverRumEvents.length).toBe(0) }) }) diff --git a/packages/rum/src/domain/rumEventsCollection/action/actionCollection.spec.ts b/packages/rum/src/domain/rumEventsCollection/action/actionCollection.spec.ts index ad0d016ab1..dcc7172add 100644 --- a/packages/rum/src/domain/rumEventsCollection/action/actionCollection.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/action/actionCollection.spec.ts @@ -99,7 +99,7 @@ describe('actionCollection v2', () => { resourceCount: 10, }, duration: 100, - id: 'xxx', + id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', name: 'foo', startTime: 1234, type: ActionType.CLICK, @@ -111,7 +111,7 @@ describe('actionCollection v2', () => { error: { count: 10, }, - id: 'xxx', + id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', loadingTime: 100 * 1e6, longTask: { count: 10, diff --git a/packages/rum/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts b/packages/rum/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts index 4d40322c9c..ceb4fefb03 100644 --- a/packages/rum/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts @@ -49,7 +49,7 @@ describe('resourceCollection', () => { category: RumEventCategory.RESOURCE, }, http: { - performance: jasmine.anything() as any, + performance: undefined, url: 'https://resource.com/valid', }, network: { @@ -258,10 +258,7 @@ describe('resourceCollection V2', () => { expect(rawRumEventsV2[0].rawRumEvent).toEqual({ date: (jasmine.any(Number) as unknown) as number, resource: { - download: jasmine.anything(), duration: 100 * 1e6, - firstByte: jasmine.anything(), - redirect: jasmine.anything(), size: undefined, type: ResourceType.OTHER, url: 'https://resource.com/valid', @@ -400,13 +397,13 @@ describe('resourceCollection V2', () => { lifeCycle.notify( LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, createResourceEntry({ - traceId: 'xxx', + traceId: '1234', }) ) const traceInfo = (rawRumEventsV2[0].rawRumEvent as RumResourceEventV2)._dd! expect(traceInfo).toBeDefined() - expect(traceInfo.traceId).toBe('xxx') + expect(traceInfo.traceId).toBe('1234') }) it('should be processed from completed request', () => { @@ -429,9 +426,21 @@ describe('resourceCollection V2', () => { function createResourceEntry(details?: Partial): RumPerformanceResourceTiming { const entry: Partial = { + connectEnd: 0, + connectStart: 0, + decodedBodySize: 0, + domainLookupEnd: 0, + domainLookupStart: 0, duration: 100, entryType: 'resource', + fetchStart: 0, name: 'https://resource.com/valid', + redirectEnd: 0, + redirectStart: 0, + requestStart: 0, + responseEnd: 0, + responseStart: 0, + secureConnectionStart: 0, startTime: 1234, ...details, } diff --git a/packages/rum/test/createRawRumEvent.ts b/packages/rum/test/createRawRumEvent.ts new file mode 100644 index 0000000000..ae68ec4e8f --- /dev/null +++ b/packages/rum/test/createRawRumEvent.ts @@ -0,0 +1,78 @@ +import { combine, Context, ErrorSource, ResourceType } from '@datadog/browser-core' +import { ActionType } from '../src/domain/rumEventsCollection/action/trackActions' +import { ViewLoadingType } from '../src/domain/rumEventsCollection/view/trackViews' +import { RawRumEventV2, RumEventType } from '../src/typesV2' + +export function createRawRumEvent(type: RumEventType, overrides?: Context): RawRumEventV2 { + switch (type) { + case RumEventType.ACTION: + return combine( + { + type, + action: { + target: { + name: 'target', + }, + type: ActionType.CUSTOM, + }, + date: 0, + }, + overrides + ) + case RumEventType.LONG_TASK: + return combine( + { + type, + date: 0, + longTask: { + duration: 0, + }, + }, + overrides + ) + case RumEventType.ERROR: + return combine( + { + type, + date: 0, + error: { + message: 'oh snap', + source: ErrorSource.SOURCE, + }, + }, + overrides + ) + case RumEventType.RESOURCE: + return combine( + { + type, + date: 0, + resource: { + duration: 0, + type: ResourceType.OTHER, + url: 'http://foo.bar', + }, + }, + overrides + ) + case RumEventType.VIEW: + return combine( + { + type, + _dd: { + documentVersion: 0, + }, + date: 0, + view: { + action: { count: 0 }, + error: { count: 0 }, + loadingType: ViewLoadingType.INITIAL_LOAD, + longTask: { count: 0 }, + resource: { count: 0 }, + timeSpent: 0, + }, + }, + overrides + ) + } +} From 4dc53a98148e14902bb936343d898fabafbe81b8 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Tue, 3 Nov 2020 15:34:39 +0100 Subject: [PATCH 4/5] fix specs --- .../action/trackActions.spec.ts | 5 +- .../view/trackViews.spec.ts | 93 +++++++++---------- 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/packages/rum/src/domain/rumEventsCollection/action/trackActions.spec.ts b/packages/rum/src/domain/rumEventsCollection/action/trackActions.spec.ts index 96de97cc61..8eb27b75cf 100644 --- a/packages/rum/src/domain/rumEventsCollection/action/trackActions.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/action/trackActions.spec.ts @@ -1,7 +1,8 @@ import { DOM_EVENT } from '@datadog/browser-core' +import { createRawRumEvent } from '../../../../test/createRawRumEvent' import { setup, TestSetupBuilder } from '../../../../test/specHelper' import { RumErrorEvent, RumEventCategory } from '../../../types' -import { RumErrorEventV2, RumEventType } from '../../../typesV2' +import { RumEventType } from '../../../typesV2' import { LifeCycle, LifeCycleEventType } from '../../lifeCycle' import { PAGE_ACTIVITY_MAX_DURATION, PAGE_ACTIVITY_VALIDATION_DELAY } from '../../trackPageActivities' import { ActionType, AutoAction } from './trackActions' @@ -202,7 +203,7 @@ describe('newAction', () => { it('counts errors occurring during the action v2', () => { const { lifeCycle, clock } = setupBuilder.build() const collectedRawRumEvent = { - rawRumEvent: ({ type: RumEventType.ERROR } as unknown) as RumErrorEventV2, + rawRumEvent: createRawRumEvent(RumEventType.ERROR), startTime: 0, } lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, pushEvent) diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts index 6dd8f67933..1e6561a927 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -1,9 +1,10 @@ +import { createRawRumEvent } from '../../../../test/createRawRumEvent' import { setup, TestSetupBuilder } from '../../../../test/specHelper' import { RumPerformanceNavigationTiming, RumPerformancePaintTiming } from '../../../browser/performanceCollection' -import { LifeCycleEventType } from '../../lifeCycle' import { RawRumEvent, RumEventCategory } from '../../../types' -import { RawRumEventV2, RumEventType } from '../../../typesV2' +import { RumEventType } from '../../../typesV2' +import { LifeCycleEventType } from '../../lifeCycle' import { PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_MAX_DURATION, @@ -70,7 +71,6 @@ describe('rum track url change', () => { let createSpy: jasmine.Spy<(event: ViewCreatedEvent) => void> beforeEach(() => { - const fakeLocation: Partial = { pathname: '/foo', hash: '' } setupBuilder = setup() .withFakeLocation('/foo') .withViewCollection() @@ -720,26 +720,19 @@ describe('rum view measures', () => { }) describe('event counts V2', () => { - function createFakeCollectedRawRumEvent(type: RumEventType) { - return { - rawRumEvent: ({ type } as unknown) as RawRumEventV2, - startTime: 0, - } - } - it('should track error count', () => { const { lifeCycle } = setupBuilder.build() expect(getHandledCount()).toEqual(1) expect(getViewEvent(0).eventCounts.errorCount).toEqual(0) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.ERROR) - ) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.ERROR) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.ERROR), + startTime: 0, + }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.ERROR), + startTime: 0, + }) history.pushState({}, '', '/bar') expect(getHandledCount()).toEqual(3) @@ -752,10 +745,10 @@ describe('rum view measures', () => { expect(getHandledCount()).toEqual(1) expect(getViewEvent(0).eventCounts.longTaskCount).toEqual(0) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.LONG_TASK) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.LONG_TASK), + startTime: 0, + }) history.pushState({}, '', '/bar') expect(getHandledCount()).toEqual(3) @@ -768,10 +761,10 @@ describe('rum view measures', () => { expect(getHandledCount()).toEqual(1) expect(getViewEvent(0).eventCounts.resourceCount).toEqual(0) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.RESOURCE) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.RESOURCE), + startTime: 0, + }) history.pushState({}, '', '/bar') expect(getHandledCount()).toEqual(3) @@ -784,10 +777,10 @@ describe('rum view measures', () => { expect(getHandledCount()).toEqual(1) expect(getViewEvent(0).eventCounts.userActionCount).toEqual(0) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.ACTION) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.ACTION), + startTime: 0, + }) history.pushState({}, '', '/bar') expect(getHandledCount()).toEqual(3) @@ -800,24 +793,24 @@ describe('rum view measures', () => { expect(getHandledCount()).toEqual(1) expect(getViewEvent(0).eventCounts.resourceCount).toEqual(0) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.RESOURCE) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.RESOURCE), + startTime: 0, + }) history.pushState({}, '', '/bar') expect(getHandledCount()).toEqual(3) expect(getViewEvent(1).eventCounts.resourceCount).toEqual(1) expect(getViewEvent(2).eventCounts.resourceCount).toEqual(0) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.RESOURCE) - ) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.RESOURCE) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.RESOURCE), + startTime: 0, + }) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.RESOURCE), + startTime: 0, + }) history.pushState({}, '', '/baz') expect(getHandledCount()).toEqual(5) @@ -835,10 +828,10 @@ describe('rum view measures', () => { userActionCount: 0, }) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.RESOURCE) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.RESOURCE), + startTime: 0, + }) expect(getHandledCount()).toEqual(1) @@ -857,10 +850,10 @@ describe('rum view measures', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() expect(getHandledCount()).toEqual(1) - lifeCycle.notify( - LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, - createFakeCollectedRawRumEvent(RumEventType.RESOURCE) - ) + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.RESOURCE), + startTime: 0, + }) expect(getHandledCount()).toEqual(1) From 7f3c95aa7a82308bb164636cc5c52e13f9f74c1f Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Tue, 3 Nov 2020 17:51:42 +0100 Subject: [PATCH 5/5] fix flaky unit tests - prevent validation on events generated by completed tests by unsubscribing rawRumEventsV2 on cleanup - ensure that mock clock is restored when all events are generated - ensure to validate early and late events --- packages/rum/src/boot/rum.spec.ts | 1 - .../longTask/longTaskCollection.spec.ts | 8 +++++++ .../view/trackViews.spec.ts | 2 +- .../view/viewCollection.spec.ts | 8 +++---- packages/rum/test/specHelper.ts | 21 ++++++++++++------- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/packages/rum/src/boot/rum.spec.ts b/packages/rum/src/boot/rum.spec.ts index 0d2fbb3451..51cdfca781 100644 --- a/packages/rum/src/boot/rum.spec.ts +++ b/packages/rum/src/boot/rum.spec.ts @@ -4,7 +4,6 @@ import { setup, TestSetupBuilder } from '../../test/specHelper' import { RumPerformanceNavigationTiming } from '../browser/performanceCollection' import { LifeCycleEventType } from '../domain/lifeCycle' -import { AutoAction } from '../domain/rumEventsCollection/action/trackActions' import { SESSION_KEEP_ALIVE_INTERVAL, THROTTLE_VIEW_UPDATE_PERIOD } from '../domain/rumEventsCollection/view/trackViews' import { RumEvent } from '../index' diff --git a/packages/rum/src/domain/rumEventsCollection/longTask/longTaskCollection.spec.ts b/packages/rum/src/domain/rumEventsCollection/longTask/longTaskCollection.spec.ts index 355d7c35e8..4763b2cf95 100644 --- a/packages/rum/src/domain/rumEventsCollection/longTask/longTaskCollection.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/longTask/longTaskCollection.spec.ts @@ -15,6 +15,10 @@ describe('long task collection', () => { }) }) + afterEach(() => { + setupBuilder.cleanup() + }) + it('should only listen to long task performance entry', () => { const { lifeCycle, rawRumEvents } = setupBuilder.build() ;[ @@ -57,6 +61,10 @@ describe('long task collection v2', () => { }) }) + afterEach(() => { + setupBuilder.cleanup() + }) + it('should only listen to long task performance entry', () => { const { lifeCycle, rawRumEventsV2 } = setupBuilder.build() ;[ diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts index 1e6561a927..ed01fa92a9 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -313,7 +313,7 @@ describe('rum track renew session', () => { }) }) -describe('rum track load duration', () => { +describe('rum track loading type', () => { let setupBuilder: TestSetupBuilder let handler: jasmine.Spy let getViewEvent: (index: number) => View diff --git a/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts index 81ed1b8552..5cef3effda 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts @@ -46,8 +46,8 @@ describe('viewCollection', () => { } lifeCycle.notify(LifeCycleEventType.VIEW_UPDATED, view as View) - expect(rawRumEvents[0].startTime).toBe(1234) - expect(rawRumEvents[0].rawRumEvent).toEqual({ + expect(rawRumEvents[rawRumEvents.length - 1].startTime).toBe(1234) + expect(rawRumEvents[rawRumEvents.length - 1].rawRumEvent).toEqual({ date: jasmine.any(Number), duration: 100 * 1e6, evt: { @@ -116,8 +116,8 @@ describe('viewCollection V2', () => { } lifeCycle.notify(LifeCycleEventType.VIEW_UPDATED, view as View) - expect(rawRumEventsV2[0].startTime).toBe(1234) - expect(rawRumEventsV2[0].rawRumEvent).toEqual({ + expect(rawRumEventsV2[rawRumEventsV2.length - 1].startTime).toBe(1234) + expect(rawRumEventsV2[rawRumEventsV2.length - 1].rawRumEvent).toEqual({ _dd: { documentVersion: 3, }, diff --git a/packages/rum/test/specHelper.ts b/packages/rum/test/specHelper.ts index 310d0020d7..7770a19c16 100644 --- a/packages/rum/test/specHelper.ts +++ b/packages/rum/test/specHelper.ts @@ -5,6 +5,7 @@ import { Configuration, Context, DEFAULT_CONFIGURATION, + noop, PerformanceObserverStubBuilder, SPEC_ENDPOINTS, withSnakeCaseKeys, @@ -82,6 +83,7 @@ export function setup(): TestSetupBuilder { } const lifeCycle = new LifeCycle() const cleanupTasks: Array<() => void> = [] + let cleanupClock = noop const beforeBuildTasks: Array<(lifeCycle: LifeCycle, configuration: Configuration, session: RumSession) => void> = [] const buildTasks: Array<() => void> = [] const rawRumEvents: Array<{ @@ -112,6 +114,13 @@ export function setup(): TestSetupBuilder { } const FAKE_APP_ID = 'appId' + // ensure that events generated before build are collected + lifeCycle.subscribe(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, (data) => rawRumEvents.push(data)) + const rawRumEventsV2Collected = lifeCycle.subscribe(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, (data) => { + rawRumEventsV2.push(data) + validateRumEventFormat(data.rawRumEvent) + }) + const setupBuilder = { withFakeLocation(initialUrl: string) { fakeLocation = buildLocation(initialUrl, location.href) @@ -221,9 +230,7 @@ export function setup(): TestSetupBuilder { const start = Date.now() spyOn(performance, 'now').and.callFake(() => Date.now() - start) clock = jasmine.clock() - cleanupTasks.push(() => { - jasmine.clock().uninstall() - }) + cleanupClock = () => jasmine.clock().uninstall() return setupBuilder }, withFakeServer() { @@ -246,11 +253,6 @@ export function setup(): TestSetupBuilder { build() { beforeBuildTasks.forEach((task) => task(lifeCycle, configuration as Configuration, session)) buildTasks.forEach((task) => task()) - lifeCycle.subscribe(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, (data) => rawRumEvents.push(data)) - lifeCycle.subscribe(LifeCycleEventType.RAW_RUM_EVENT_V2_COLLECTED, (data) => { - rawRumEventsV2.push(data) - validateRumEventFormat(data.rawRumEvent) - }) return { clock, fakeLocation, @@ -269,6 +271,9 @@ export function setup(): TestSetupBuilder { }, cleanup() { cleanupTasks.forEach((task) => task()) + // perform these steps at the end to generate correct events in cleanup and validate them + cleanupClock() + rawRumEventsV2Collected.unsubscribe() }, } return setupBuilder