test(tracer): make e2e tests to follow the same convention in Logger and Metrics #788

ijemmy · 2022-04-20T15:02:30Z

Description of your changes

Refactor tracer traces to be the same as other utilities.
- Test infra are split from one stack to 4 different stack (for each test file)
- Take annotation and metadata test out of the existing test cases
- Create helper function to abstract some details to improve readability
Make stack names unique (Maintenance: make e2e test stack name unique #497 )
Support running in multiple runtime

How to verify this change

Run e2e tests in workflow
Run this command locally

 AWS_PROFILE=YOUR_PROFILE AWS_REGION=eu-west-1 npx jest --group=e2e/tracer

Results (partial)


[test:e2e:nodejs12x]     console.log
[test:e2e:nodejs12x]       Manual query: aws xray get-trace-summaries --start-time 1650468060 --end-time 1650468169 --filter-expression 'resource.arn = "arn:aws:lambda:eu-west-1:561912387782:function:Tracer-E2E-nodejs12x-AllFeatures-Decorator-TracerDisabled-f19a6b"'
[test:e2e:nodejs12x]
[test:e2e:nodejs12x]       at tests/helpers/tracesUtils.ts:89:13
[test:e2e:nodejs12x]
[test:e2e:nodejs12x]
[test:e2e:nodejs12x] Test Suites: 4 passed, 4 of 9 total
[test:e2e:nodejs12x] Tests:       12 passed, 12 total
[test:e2e:nodejs12x] Snapshots:   0 total
[test:e2e:nodejs12x] Time:        197.391 s
[test:e2e:nodejs12x] Ran all test suites.
[test:e2e:nodejs12x] npm run test:e2e:nodejs12x exited with code 0

Related issues, RFCs

#497
#529

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

Breaking change checklist

I have documented the migration process
I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ambda function

ijemmy · 2022-04-20T15:10:07Z

packages/tracing/tests/e2e/allFeatures.decorator.test.ts

+  it('should have correct annotations and metadata', async () => {
+    const tracesWhenAllFlagsEnabled = await getTraces(xray, startTime, await getFunctionArn(functionNameWithAllFlagsEnabled), invocations, 5);
+
+    for (let i = 0; i < invocations; i++) {
+      const trace = tracesWhenAllFlagsEnabled[i];
+      const invocationSubsegment = getInvocationSubsegment(trace);
+      const handlerSubsegment = getFirstSubsegment(invocationSubsegment);
+      const { annotations, metadata } = handlerSubsegment;
+
+      const isColdStart = (i === 0);
+      assertAnnotation({
+        annotations,
+        isColdStart,
+        expectedServiceName: serviceNameWithAllFlagsEnabled,
+        expectedCustomAnnotationKey,
+        expectedCustomAnnotationValue,
+      });
+
+      if (!metadata) {
+        fail('metadata is missing');
+      }
+      expect(metadata[serviceNameWithAllFlagsEnabled][expectedCustomMetadataKey])
+        .toEqual(expectedCustomMetadataValue);
+
+      const shouldThrowAnError = (i === (invocations - 1));
+      if (!shouldThrowAnError) {
+        // Assert that the metadata object contains the response
+        expect(metadata[serviceNameWithAllFlagsEnabled]['index.handler response'])
+          .toEqual(expectedCustomResponseValue);
+      }
+    }
+  }, TEST_CASE_TIMEOUT);


I break this part out of the existing test case to make it not too long.

ijemmy · 2022-04-20T15:11:18Z

packages/tracing/tests/e2e/allFeatures.decorator.test.ts

+       * 3. httpbin.org (Remote call)
+       * 4. '### myMethod' (method decorator)
+       */
+      const handlerSubsegment = getFirstSubsegment(invocationSubsegment);


Instead of nested if. I use a helper function to ensure that the item isn't null. The check to please TypeScript compilers are moved to the helper method.

Love this, thanks for simplyfing that indentation hell that I got us in :D

ijemmy · 2022-04-20T15:13:30Z

packages/tracing/tests/e2e/allFeatures.decorator.test.ts

+      const shouldThrowAnError = (i === (invocations - 1));
+      if (shouldThrowAnError) {
+        // Assert that the subsegment has the expected fault
+        expect(invocationSubsegment.error).toBe(true);
+        expect(handlerSubsegment.error).toBe(true);
+        // Assert that no error was captured on the subsegment
+        expect(handlerSubsegment.hasOwnProperty('cause')).toBe(false);
+      }


This is the only unique part of this test case.

ijemmy · 2022-04-20T15:15:46Z

packages/tracing/tests/e2e/allFeatures.middy.test.ts

+/**
+ * Test tracer in middy setup
+ *
+ * @group e2e/tracer/middy
+ */
+


This is almost the same as decorator case. The only difference is that we do not captureMethod() so we have 1 less custom trace.

ijemmy · 2022-04-21T07:29:31Z

e2e workflow run: https://github.com/awslabs/aws-lambda-powertools-typescript/actions/runs/2196694113

dreamorosi · 2022-04-21T12:33:15Z

packages/tracing/tests/e2e/asyncHandler.decorator.test.ts

+import {
+  generateUniqueName,
+  isValidRuntimeKey,
+} from '@aws-lambda-powertools/commons/tests/utils/e2eUtils';


extra-nit: can we swap this import and the previous one so we have all the relative ones after the one from node_modules?

This is a mistake. I intend to import relatively, not from the package. (or we need to rebuild common for every change)

I'll change this (and other files) to relative import.

dreamorosi

Left one extra-minor comment about module import order but approving already since it's purely stylistic.

Really like the new test setup, a breadth of fresh air :)

dreamorosi

LGTM

ijemmy added 10 commits April 14, 2022 17:43

test(tracer): refactor to pass runtime info + extract code to add a L…

849e890

…ambda function

test(tracer): refactor to extract common used values out

0f1e56c

test: refactor to take manual test out in its own separated file

33e8c70

test(tracer): refactor to move reusable code to functions

1f5e8ea

test(tracer): add middy test + refactor to make it more readable

8e86295

test(tracer): Add two variant of middy case

db3ee54

test(tracer): Add decorator cases

90c455a

test:(tracer): add async handler test cases

8516b88

test:(tracer) remove unused code

6f4e50e

test(tracer): update comment + fix incorrect test case name

9f4391c

ijemmy commented Apr 20, 2022

View reviewed changes

test(tracer): add targets to run test in both runtimes

6e571a3

ijemmy requested review from dreamorosi, flochaz and saragerion April 20, 2022 15:29

ijemmy marked this pull request as ready for review April 20, 2022 15:29

ijemmy changed the title ~~test(tracer): refactor e2e tests to follow the same convention in Logger and Metrics~~ test(tracer): make e2e tests to follow the same convention in Logger and Metrics Apr 21, 2022

dreamorosi assigned ijemmy Apr 21, 2022

dreamorosi added internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) tracer This item relates to the Tracer Utility labels Apr 21, 2022

dreamorosi added this to the production-ready-release milestone Apr 21, 2022

dreamorosi linked an issue Apr 21, 2022 that may be closed by this pull request

Maintenance: make e2e test stack name unique #497

Closed

dreamorosi reviewed Apr 21, 2022

View reviewed changes

dreamorosi previously approved these changes Apr 21, 2022

View reviewed changes

test(tracer): make import relative and put relative imports after

73e8e87

ijemmy dismissed dreamorosi’s stale review via 73e8e87 April 22, 2022 12:25

dreamorosi approved these changes Apr 25, 2022

View reviewed changes

flochaz approved these changes Apr 29, 2022

View reviewed changes

dreamorosi merged commit a47c94c into main Apr 29, 2022

dreamorosi deleted the ijemmy/refactor-e2e-tests-tracer branch April 29, 2022 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(tracer): make e2e tests to follow the same convention in Logger and Metrics #788

test(tracer): make e2e tests to follow the same convention in Logger and Metrics #788

ijemmy commented Apr 20, 2022 •

edited

Loading

ijemmy Apr 20, 2022

ijemmy Apr 20, 2022

dreamorosi Apr 21, 2022

ijemmy Apr 20, 2022

ijemmy Apr 20, 2022

ijemmy commented Apr 21, 2022

dreamorosi Apr 21, 2022

ijemmy Apr 22, 2022

dreamorosi left a comment

dreamorosi left a comment

test(tracer): make e2e tests to follow the same convention in Logger and Metrics #788

test(tracer): make e2e tests to follow the same convention in Logger and Metrics #788

Conversation

ijemmy commented Apr 20, 2022 • edited Loading

Description of your changes

How to verify this change

Related issues, RFCs

PR status

Checklist

Breaking change checklist

ijemmy Apr 20, 2022

Choose a reason for hiding this comment

ijemmy Apr 20, 2022

Choose a reason for hiding this comment

dreamorosi Apr 21, 2022

Choose a reason for hiding this comment

ijemmy Apr 20, 2022

Choose a reason for hiding this comment

ijemmy Apr 20, 2022

Choose a reason for hiding this comment

ijemmy commented Apr 21, 2022

dreamorosi Apr 21, 2022

Choose a reason for hiding this comment

ijemmy Apr 22, 2022

Choose a reason for hiding this comment

dreamorosi left a comment

Choose a reason for hiding this comment

dreamorosi left a comment

Choose a reason for hiding this comment

ijemmy commented Apr 20, 2022 •

edited

Loading