Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✅ improve async calls collection #2123

Merged
merged 1 commit into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/core/src/transport/httpRequest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,12 @@ describe('httpRequest', () => {
return Promise.resolve({ status: 200 })
}
})
const { waitAsyncCalls, expectNoExtraAsyncCall } = collectAsyncCalls(fetchSpy)

interceptor.withFetch(fetchSpy)

request.send({ data: '{"foo":"bar1"}\n{"foo":"bar2"}', bytesCount: 10 })

waitAsyncCalls(2, () => expectNoExtraAsyncCall(done))
collectAsyncCalls(fetchSpy, 2, () => done())
})
})

Expand Down
43 changes: 25 additions & 18 deletions packages/core/test/collectAsyncCalls.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,30 @@
export function collectAsyncCalls<F extends jasmine.Func>(spy: jasmine.Spy<F>) {
return {
waitAsyncCalls: (expectedCallsCount: number, callback: (calls: jasmine.Calls<F>) => void) => {
import { getCurrentJasmineSpec } from './getCurrentJasmineSpec'

export function collectAsyncCalls<F extends jasmine.Func>(
spy: jasmine.Spy<F>,
expectedCallsCount: number,
callback: (calls: jasmine.Calls<F>) => void
) {
const currentSpec = getCurrentJasmineSpec()
if (!currentSpec) {
throw new Error('collectAsyncCalls should be called within jasmine code')
}

if (spy.calls.count() === expectedCallsCount) {
spy.and.callFake(extraCallDetected as F)
callback(spy.calls)
} else if (spy.calls.count() > expectedCallsCount) {
extraCallDetected()
} else {
spy.and.callFake((() => {
if (spy.calls.count() === expectedCallsCount) {
spy.and.callFake(extraCallDetected as F)
callback(spy.calls)
} else if (spy.calls.count() > expectedCallsCount) {
fail('Unexpected extra call')
} else {
spy.and.callFake((() => {
if (spy.calls.count() === expectedCallsCount) {
callback(spy.calls)
}
}) as F)
}
},
expectNoExtraAsyncCall: (done: () => void) => {
spy.and.callFake((() => {
fail('Unexpected extra call')
}) as F)
setTimeout(done, 300)
},
}) as F)
}

function extraCallDetected() {
fail(`Unexpected extra call for spec '${currentSpec!.fullName}'`)
}
}
14 changes: 14 additions & 0 deletions packages/core/test/getCurrentJasmineSpec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
let currentSpec: jasmine.SpecResult | null = null

export function getCurrentJasmineSpec() {
return currentSpec
}

jasmine.getEnv().addReporter({
specStarted(specResult) {
currentSpec = specResult
},
specDone() {
currentSpec = null
},
})
57 changes: 25 additions & 32 deletions packages/rum/src/boot/startRecording.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,9 @@ describe('startRecording', () => {
let setupBuilder: TestSetupBuilder
let sessionManager: RumSessionManagerMock
let viewId: string
let waitRequestSendCalls: (
expectedCallsCount: number,
callback: (calls: jasmine.Calls<HttpRequest['sendOnExit']>) => void
) => void
let sandbox: HTMLElement
let textField: HTMLInputElement
let expectNoExtraRequestSendCalls: (done: () => void) => void
let requestSendSpy: jasmine.Spy<HttpRequest['sendOnExit']>
let stopRecording: () => void

beforeEach((done) => {
Expand Down Expand Up @@ -55,15 +51,12 @@ describe('startRecording', () => {
defaultPrivacyLevel: DefaultPrivacyLevel.ALLOW,
})
.beforeBuild(({ lifeCycle, configuration, viewContexts, sessionManager }) => {
const requestSendSpy = jasmine.createSpy()
const httpRequest: HttpRequest = {
requestSendSpy = jasmine.createSpy()
const httpRequest = {
send: requestSendSpy,
sendOnExit: requestSendSpy,
}

;({ waitAsyncCalls: waitRequestSendCalls, expectNoExtraAsyncCall: expectNoExtraRequestSendCalls } =
collectAsyncCalls(requestSendSpy))

const recording = startRecording(lifeCycle, configuration, sessionManager, viewContexts, worker!, httpRequest)
stopRecording = recording ? recording.stop : noop
return { stop: stopRecording }
Expand All @@ -82,7 +75,7 @@ describe('startRecording', () => {
const { lifeCycle } = setupBuilder.build()
flushSegment(lifeCycle)

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
expect(calls.first().args[0]).toEqual({ data: jasmine.any(FormData), bytesCount: jasmine.any(Number) })
expect(getRequestData(calls.first())).toEqual({
'application.id': 'appId',
Expand All @@ -98,7 +91,7 @@ describe('startRecording', () => {
index_in_view: '0',
source: 'browser',
})
expectNoExtraRequestSendCalls(done)
done()
})
})

Expand All @@ -113,9 +106,9 @@ describe('startRecording', () => {
document.body.dispatchEvent(inputEvent)
}

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
expect(getRequestData(calls.first()).records_count).toBe(String(inputCount + recordsPerFullSnapshot()))
expectNoExtraRequestSendCalls(done)
done()
})
})

Expand All @@ -130,9 +123,9 @@ describe('startRecording', () => {

flushSegment(lifeCycle)

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
expect(getRequestData(calls.first()).records_count).toBe(String(1 + recordsPerFullSnapshot()))
expectNoExtraRequestSendCalls(done)
done()
})
})

Expand All @@ -148,11 +141,11 @@ describe('startRecording', () => {

flushSegment(lifeCycle)

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
const data = getRequestData(calls.first())
expect(data.records_count).toBe('1')
expect(data['session.id']).toBe('new-session-id')
expectNoExtraRequestSendCalls(done)
done()
})
})

Expand All @@ -162,9 +155,9 @@ describe('startRecording', () => {
changeView(lifeCycle)
flushSegment(lifeCycle)

waitRequestSendCalls(2, (calls) => {
collectAsyncCalls(requestSendSpy, 2, (calls) => {
expect(getRequestData(calls.mostRecent()).has_full_snapshot).toBe('true')
expectNoExtraRequestSendCalls(done)
done()
})
})

Expand All @@ -175,7 +168,7 @@ describe('startRecording', () => {
changeView(lifeCycle)
flushSegment(lifeCycle)

waitRequestSendCalls(2, (calls) => {
collectAsyncCalls(requestSendSpy, 2, (calls) => {
readRequestSegment(calls.first(), (segment) => {
expect(segment.records[0].timestamp).toEqual(timeStampNow())
expect(segment.records[1].timestamp).toEqual(timeStampNow())
Expand All @@ -189,7 +182,7 @@ describe('startRecording', () => {
expect(segment.records[1].timestamp).toEqual(VIEW_TIMESTAMP)
expect(segment.records[2].timestamp).toEqual(VIEW_TIMESTAMP)

expectNoExtraRequestSendCalls(done)
done()
})
})
})
Expand All @@ -201,11 +194,11 @@ describe('startRecording', () => {
changeView(lifeCycle)
flushSegment(lifeCycle)

waitRequestSendCalls(2, (calls) => {
collectAsyncCalls(requestSendSpy, 2, (calls) => {
expect(getRequestData(calls.first())['view.id']).toBe('view-id')
readRequestSegment(calls.first(), (segment) => {
expect(segment.records[segment.records.length - 1].type).toBe(RecordType.ViewEnd)
expectNoExtraRequestSendCalls(done)
done()
})
})
})
Expand All @@ -217,14 +210,14 @@ describe('startRecording', () => {
changeView(lifeCycle)
flushSegment(lifeCycle)

waitRequestSendCalls(2, (calls) => {
collectAsyncCalls(requestSendSpy, 2, (calls) => {
readRequestSegment(calls.first(), (segment) => {
expect(segment.records[segment.records.length - 2].type).toBe(RecordType.IncrementalSnapshot)
expect(segment.records[segment.records.length - 1].type).toBe(RecordType.ViewEnd)

readRequestSegment(calls.mostRecent(), (segment) => {
expect(segment.records[0].type).toBe(RecordType.Meta)
expectNoExtraRequestSendCalls(done)
done()
})
})
})
Expand All @@ -234,12 +227,12 @@ describe('startRecording', () => {
setSegmentBytesLimit(0)
setupBuilder.build()

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
readRequestSegment(calls.first(), (segment) => {
expect(segment.records[0].type).toBe(RecordType.Meta)
expect(segment.records[1].type).toBe(RecordType.Focus)
expect(segment.records[2].type).toBe(RecordType.FullSnapshot)
expectNoExtraRequestSendCalls(done)
done()
})
})
})
Expand All @@ -253,9 +246,9 @@ describe('startRecording', () => {
document.body.dispatchEvent(createNewEvent('click', { clientX: 1, clientY: 2 }))
flushSegment(lifeCycle)

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
expect(getRequestData(calls.first()).records_count).toBe(String(1 + recordsPerFullSnapshot()))
expectNoExtraRequestSendCalls(done)
done()
})
})

Expand All @@ -266,9 +259,9 @@ describe('startRecording', () => {
changeView(lifeCycle)
flushSegment(lifeCycle)

waitRequestSendCalls(1, (calls) => {
collectAsyncCalls(requestSendSpy, 1, (calls) => {
expect(getRequestData(calls.first()).records_count).toBe(String(recordsPerFullSnapshot()))
expectNoExtraRequestSendCalls(done)
done()
})
})
})
Expand Down
9 changes: 2 additions & 7 deletions packages/rum/src/domain/record/mutationBatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,9 @@ describe('createMutationBatch', () => {
const mutation = { type: 'childList' } as RumMutationRecord
mutationBatch.addMutations([mutation])

const {
waitAsyncCalls: waitProcessMutationBatchCalls,
expectNoExtraAsyncCall: expectNoExtraProcessMutationBatchCall,
} = collectAsyncCalls(processMutationBatchSpy)

waitProcessMutationBatchCalls(1, (calls) => {
collectAsyncCalls(processMutationBatchSpy, 1, (calls) => {
expect(calls.mostRecent().args[0]).toEqual([mutation])
expectNoExtraProcessMutationBatchCall(done)
done()
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,12 @@ describe('startMutationCollection', () => {
it('processes mutations asynchronously', (done) => {
serializeDocumentWithDefaults()
const { mutationCallbackSpy } = startMutationCollection()
const { waitAsyncCalls: waitMutationCallbackCalls, expectNoExtraAsyncCall: expectNoExtraMutationCallbackCalls } =
collectAsyncCalls(mutationCallbackSpy)

sandbox.appendChild(document.createElement('div'))

expect(mutationCallbackSpy).not.toHaveBeenCalled()

waitMutationCallbackCalls(1, () => {
expectNoExtraMutationCallbackCalls(done)
})
collectAsyncCalls(mutationCallbackSpy, 1, () => done())
})

it('does not emit a mutation when a node is appended to a unknown node', () => {
Expand Down
11 changes: 4 additions & 7 deletions packages/rum/src/domain/record/record.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ describe('record', () => {
let sandbox: HTMLElement
let recordApi: RecordAPI
let emitSpy: jasmine.Spy<(record: BrowserRecord) => void>
let waitEmitCalls: (expectedCallsCount: number, callback: () => void) => void
let expectNoExtraEmitCalls: (done: () => void) => void
let clock: Clock | undefined

beforeEach(() => {
Expand All @@ -30,7 +28,6 @@ describe('record', () => {
}

emitSpy = jasmine.createSpy()
;({ waitAsyncCalls: waitEmitCalls, expectNoExtraAsyncCall: expectNoExtraEmitCalls } = collectAsyncCalls(emitSpy))
sandbox = createDOMSandbox()
})

Expand Down Expand Up @@ -60,7 +57,7 @@ describe('record', () => {
styleSheet.insertRule('body { color: #ccc; }')
}, 10)

waitEmitCalls(recordsPerFullSnapshot() + 6, () => {
collectAsyncCalls(emitSpy, recordsPerFullSnapshot() + 6, () => {
const records = getEmittedRecords()
let i = 0

Expand Down Expand Up @@ -115,7 +112,7 @@ describe('record', () => {
})
)

expectNoExtraEmitCalls(done)
done()
})
})

Expand All @@ -126,7 +123,7 @@ describe('record', () => {

recordApi.takeSubsequentFullSnapshot()

waitEmitCalls(1 + 2 * recordsPerFullSnapshot(), () => {
collectAsyncCalls(emitSpy, 1 + 2 * recordsPerFullSnapshot(), () => {
const records = getEmittedRecords()
let i = 0

Expand All @@ -143,7 +140,7 @@ describe('record', () => {
expect(records[i++].type).toEqual(RecordType.Focus)
expect(records[i++].type).toEqual(RecordType.FullSnapshot)

expectNoExtraEmitCalls(done)
done()
})
})

Expand Down