From 8e5d9adc8ab2b8e67a33d2387dff4a7483cfc3e1 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 3 Oct 2022 09:45:18 -0700 Subject: [PATCH 1/6] chore: added unit test for captureMethod async/await --- packages/tracer/tests/unit/Tracer.test.ts | 50 ++++++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 4e77ebf490..82f226a7eb 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -15,10 +15,12 @@ interface LambdaInterface { } type CaptureAsyncFuncMock = jest.SpyInstance unknown, parent?: Segment | Subsegment]>; -const createCaptureAsyncFuncMock = function(provider: ProviderServiceInterface): CaptureAsyncFuncMock { +const createCaptureAsyncFuncMock = function(provider: ProviderServiceInterface, subsegment?: Subsegment): CaptureAsyncFuncMock { return jest.spyOn(provider, 'captureAsyncFunc') .mockImplementation(async (methodName, callBackFn) => { - const subsegment = new Subsegment(`### ${methodName}`); + if (!subsegment) { + subsegment = new Subsegment(`### ${methodName}`); + } jest.spyOn(subsegment, 'flush').mockImplementation(() => null); await callBackFn(subsegment); }); @@ -1239,6 +1241,50 @@ describe('Class: Tracer', () => { }); + test('when used as decorator on an async method, the method is awaited correctly', async () => { + + // Prepare + const tracer: Tracer = new Tracer(); + const newSubsegment: Segment | Subsegment | undefined = new Subsegment('### dummyMethod'); + + jest.spyOn(tracer.provider, 'getSegment') + .mockImplementation(() => newSubsegment); + setContextMissingStrategy(() => null); + const subsegmentCloseSpy = jest.spyOn(newSubsegment, 'close').mockImplementation(); + createCaptureAsyncFuncMock(tracer.provider, newSubsegment); + + class Lambda implements LambdaInterface { + @tracer.captureMethod() + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + public async dummyMethod(): Promise { + return; + } + + public async handler(_event: TEvent, _context: Context, _callback: Callback): Promise { + await this.dummyMethod(); + this.otherDummyMethod(); + + return; + } + + public otherDummyMethod(): void { + return; + } + + } + + // Act + const lambda = new Lambda(); + const otherDummyMethodSpy = jest.spyOn(lambda, 'otherDummyMethod').mockImplementation(); + const handler = lambda.handler.bind(lambda); + await handler({}, context, () => console.log('Lambda invoked!')); + + // Assess + expect(subsegmentCloseSpy.mock.invocationCallOrder[0]).toBeLessThan(otherDummyMethodSpy.mock.invocationCallOrder[0]); + + }); + }); describe('Method: captureAWS', () => { From ea6e0b7d2ce633bbc126d09276356d5b425355ca Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 3 Oct 2022 09:49:30 -0700 Subject: [PATCH 2/6] chore: added unit test for captureLambdaHandler async/await --- packages/tracer/tests/unit/Tracer.test.ts | 45 +++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 82f226a7eb..68bf5edfd8 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -958,6 +958,51 @@ describe('Class: Tracer', () => { expect(await handler({}, context, () => console.log('Lambda invoked!'))).toEqual('memberVariable:someValue'); }); + + test('when used as decorator on an async method, the method is awaited correctly', async () => { + + // Prepare + const tracer: Tracer = new Tracer(); + const newSubsegment: Segment | Subsegment | undefined = new Subsegment('### dummyMethod'); + + jest.spyOn(tracer.provider, 'getSegment') + .mockImplementation(() => newSubsegment); + setContextMissingStrategy(() => null); + const subsegmentCloseSpy = jest.spyOn(newSubsegment, 'close').mockImplementation(); + createCaptureAsyncFuncMock(tracer.provider, newSubsegment); + + class Lambda implements LambdaInterface { + public async dummyMethod(): Promise { + return; + } + + @tracer.captureLambdaHandler() + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + public async handler(_event: TEvent, _context: Context, _callback: Callback): void | Promise { + await this.dummyMethod(); + this.otherDummyMethod(); + + return; + } + + public otherDummyMethod(): void { + return; + } + + } + + // Act + const lambda = new Lambda(); + const otherDummyMethodSpy = jest.spyOn(lambda, 'otherDummyMethod').mockImplementation(); + const handler = lambda.handler.bind(lambda); + await handler({}, context, () => console.log('Lambda invoked!')); + + // Assess + expect(otherDummyMethodSpy.mock.invocationCallOrder[0]).toBeLessThan(subsegmentCloseSpy.mock.invocationCallOrder[0]); + + }); + }); describe('Method: captureMethod', () => { From a23d8691a7b0bc36885c98403f32d029e7297ab2 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 3 Oct 2022 09:52:22 -0700 Subject: [PATCH 3/6] chore: added comments to document unit test cases --- packages/tracer/tests/unit/Tracer.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 68bf5edfd8..492efaef49 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -999,6 +999,9 @@ describe('Class: Tracer', () => { await handler({}, context, () => console.log('Lambda invoked!')); // Assess + // Here we assert that the otherDummyMethodSpy method is called before the cleanup logic (inside the finally of decorator) + // that should always be called after the handler has returned. If otherDummyMethodSpy is called after it means the + // decorator is NOT awaiting the handler which would cause the test to fail. expect(otherDummyMethodSpy.mock.invocationCallOrder[0]).toBeLessThan(subsegmentCloseSpy.mock.invocationCallOrder[0]); }); @@ -1326,6 +1329,9 @@ describe('Class: Tracer', () => { await handler({}, context, () => console.log('Lambda invoked!')); // Assess + // Here we assert that the subsegment.close() (inside the finally of decorator) is called before the other otherDummyMethodSpy method + // that should always be called after the handler has returned. If subsegment.close() is called after it means the + // decorator is NOT awaiting the method which would cause the test to fail. expect(subsegmentCloseSpy.mock.invocationCallOrder[0]).toBeLessThan(otherDummyMethodSpy.mock.invocationCallOrder[0]); }); From 136a63a189181e3c0d7f44ca8a3fc86c845a0cc1 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Thu, 6 Oct 2022 06:55:24 -0700 Subject: [PATCH 4/6] Update packages/tracer/tests/unit/Tracer.test.ts Co-authored-by: ijemmy --- packages/tracer/tests/unit/Tracer.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 13b04d76b6..37e51e5f10 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -1330,7 +1330,9 @@ describe('Class: Tracer', () => { // Here we assert that the subsegment.close() (inside the finally of decorator) is called before the other otherDummyMethodSpy method // that should always be called after the handler has returned. If subsegment.close() is called after it means the // decorator is NOT awaiting the method which would cause the test to fail. - expect(subsegmentCloseSpy.mock.invocationCallOrder[0]).toBeLessThan(otherDummyMethodSpy.mock.invocationCallOrder[0]); +const dummyCallOrder = subsegmentCloseSpy.mock.invocationCallOrder[0]; +const otherDummyCallOrder = otherDummyMethodSpy.mock.invocationCallOrder[0]; +expect(dummyCallOrder).toBeLessThan(otherDummyCallOrder); }); From 542d13389bad7f739d78fc854b7775d409ba6684 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Thu, 6 Oct 2022 06:55:30 -0700 Subject: [PATCH 5/6] Update packages/tracer/tests/unit/Tracer.test.ts Co-authored-by: ijemmy --- packages/tracer/tests/unit/Tracer.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 37e51e5f10..6cf7f42c88 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -1002,7 +1002,9 @@ describe('Class: Tracer', () => { // Here we assert that the otherDummyMethodSpy method is called before the cleanup logic (inside the finally of decorator) // that should always be called after the handler has returned. If otherDummyMethodSpy is called after it means the // decorator is NOT awaiting the handler which would cause the test to fail. - expect(otherDummyMethodSpy.mock.invocationCallOrder[0]).toBeLessThan(subsegmentCloseSpy.mock.invocationCallOrder[0]); +const dummyCallOrder = subsegmentCloseSpy.mock.invocationCallOrder[0]; +const otherDummyCallOrder = otherDummyMethodSpy.mock.invocationCallOrder[0]; +expect(dummyCallOrder).toBeMoreThan(otherDummyCallOrder); }); From 531223c248fbc0ad6a6b932b09fc26e0b6c8621a Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Thu, 6 Oct 2022 07:06:02 -0700 Subject: [PATCH 6/6] chore: fix linting --- packages/tracer/tests/unit/Tracer.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 6cf7f42c88..1bf5fcd358 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -1002,9 +1002,9 @@ describe('Class: Tracer', () => { // Here we assert that the otherDummyMethodSpy method is called before the cleanup logic (inside the finally of decorator) // that should always be called after the handler has returned. If otherDummyMethodSpy is called after it means the // decorator is NOT awaiting the handler which would cause the test to fail. -const dummyCallOrder = subsegmentCloseSpy.mock.invocationCallOrder[0]; -const otherDummyCallOrder = otherDummyMethodSpy.mock.invocationCallOrder[0]; -expect(dummyCallOrder).toBeMoreThan(otherDummyCallOrder); + const dummyCallOrder = subsegmentCloseSpy.mock.invocationCallOrder[0]; + const otherDummyCallOrder = otherDummyMethodSpy.mock.invocationCallOrder[0]; + expect(otherDummyCallOrder).toBeLessThan(dummyCallOrder); }); @@ -1332,9 +1332,9 @@ expect(dummyCallOrder).toBeMoreThan(otherDummyCallOrder); // Here we assert that the subsegment.close() (inside the finally of decorator) is called before the other otherDummyMethodSpy method // that should always be called after the handler has returned. If subsegment.close() is called after it means the // decorator is NOT awaiting the method which would cause the test to fail. -const dummyCallOrder = subsegmentCloseSpy.mock.invocationCallOrder[0]; -const otherDummyCallOrder = otherDummyMethodSpy.mock.invocationCallOrder[0]; -expect(dummyCallOrder).toBeLessThan(otherDummyCallOrder); + const dummyCallOrder = subsegmentCloseSpy.mock.invocationCallOrder[0]; + const otherDummyCallOrder = otherDummyMethodSpy.mock.invocationCallOrder[0]; + expect(dummyCallOrder).toBeLessThan(otherDummyCallOrder); });