chore(tests): address integration test flakiness (#1669)

dreamorosi · web-flow · commit f3916d158e07 · 2023-09-04T10:40:31.000+02:00
* chore: traces retrieval

* chore: secure string uniqueness

* chore: update e2e command

* chore: update test names

* chore: remove layer flag

* chore: remove layer flag

* chore: remove STS client from devDeps tracer

* chore: restore retryOptions
diff --git a/.github/workflows/run-e2e-tests.yml b/.github/workflows/run-e2e-tests.yml
@@ -19,7 +19,7 @@ jobs:
       contents: read
     strategy:
       matrix:
-        package: [logger, metrics, tracer, parameters, idempotency]
+        package: [layers, packages/logger, packages/metrics, packages/tracer, packages/parameters, packages/idempotency]
         version: [14, 16, 18]
       fail-fast: false
     steps:
@@ -55,56 +55,14 @@ jobs:
           role-to-assume: ${{ secrets.AWS_ROLE_ARN_TO_ASSUME }}
           aws-region: eu-west-1
           mask-aws-account-id: true
-      - name: Run integration tests on utils
-        run: |
-          RUNTIME=nodejs${{ matrix.version }}x npm run test:e2e -w packages/${{ matrix.package }}
-  layer-e2e-tests:
-    runs-on: ubuntu-latest
-    env:
-      NODE_ENV: dev
-      PR_NUMBER: ${{ inputs.prNumber }}
-    permissions:
-      id-token: write # needed to interact with GitHub's OIDC Token endpoint.
-      contents: read
-    strategy:
-      fail-fast: false
-      matrix:
-        version: [14, 16, 18]
-    steps:
-      - name: Checkout Repo
-        uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab  # v3.5.2
-      # If we pass a PR Number when triggering the workflow we will retrieve the PR info and get its headSHA
-      - name: Extract PR details
-        id: extract_PR_details
-        if: ${{ inputs.prNumber != '' }}
-        uses: actions/github-script@d7906e4ad0b1822421a7e6a35d5ca353c962f410 # v6.4.1
-        with:
-          script: |
-            const script = require('.github/scripts/get_pr_info.js');
-            await script({github, context, core});
-      # Only if a PR Number was passed and the headSHA of the PR extracted,
-      # we checkout the PR at that point in time
-      - name: Checkout PR code
-        if: ${{ inputs.prNumber != '' }}
-        uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab  # v3.5.2
-        with:
-          ref: ${{ steps.extract_PR_details.outputs.headSHA }}
-      - name: Setup NodeJS
-        uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
-        with:
-          # Always use version 18
-          node-version: 18
-      - name: Setup dependencies
-        uses: ./.github/actions/cached-node-modules
-      - name: "Configure AWS credentials"
-        uses: aws-actions/configure-aws-credentials@04b98b3f9e85f563fb061be8751a0352327246b0 # v3.0.1
-        with:
-          role-to-assume: ${{ secrets.AWS_ROLE_ARN_TO_ASSUME }}
-          aws-region: eu-west-1
-          mask-aws-account-id: true
       - name: Create layer files
+        if: ${{ matrix.package == 'layers' }}
         run: |
           export VERSION=latest
           bash .github/scripts/setup_tmp_layer_files.sh
-      - name: Run integration test on layers
-        run: RUNTIME=nodejs${{ matrix.version }}x npm run test:e2e -w layers
+      - name: Run integration tests on utils
+        env:
+          RUNTIME: nodejs${{ matrix.version }}x
+          CI: true
+          JSII_SILENCE_WARNING_DEPRECATED_NODE_VERSION: true
+        run: npm run test:e2e -w ${{ matrix.package }}
diff --git a/layers/src/layer-publisher-stack.ts b/layers/src/layer-publisher-stack.ts
@@ -13,7 +13,6 @@ export interface LayerPublisherStackProps extends StackProps {
   readonly layerName?: string;
   readonly powertoolsPackageVersion?: string;
   readonly ssmParameterLayerArn: string;
-  readonly removeLayerVersion?: boolean;
 }
 
 export class LayerPublisherStack extends Stack {
@@ -25,7 +24,7 @@ export class LayerPublisherStack extends Stack {
   ) {
     super(scope, id, props);
 
-    const { layerName, powertoolsPackageVersion, removeLayerVersion } = props;
+    const { layerName, powertoolsPackageVersion } = props;
 
     console.log(
       `publishing layer ${layerName} version : ${powertoolsPackageVersion}`
@@ -43,10 +42,6 @@ export class LayerPublisherStack extends Stack {
       // This is needed because the following regions do not support the compatibleArchitectures property #1400
       // ...(![ 'eu-south-2', 'eu-central-2', 'ap-southeast-4' ].includes(Stack.of(this).region) ? { compatibleArchitectures: [Architecture.X86_64] } : {}),
       code: Code.fromAsset(resolve(__dirname, '..', '..', 'tmp')),
-      removalPolicy:
-        removeLayerVersion === true
-          ? RemovalPolicy.DESTROY
-          : RemovalPolicy.RETAIN,
     });
 
     const layerPermission = new CfnLayerVersionPermission(
diff --git a/layers/tests/e2e/layerPublisher.test.ts b/layers/tests/e2e/layerPublisher.test.ts
@@ -61,7 +61,6 @@ describe(`Layers E2E tests, publisher stack`, () => {
     layerName: layerId,
     powertoolsPackageVersion: powerToolsPackageVersion,
     ssmParameterLayerArn: ssmParameterLayerName,
-    removeLayerVersion: true,
   });
   const testLayerStack = new TestStack({
     stackNameProps: {
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/logger/tests/e2e/basicFeatures.middy.test.ts b/packages/logger/tests/e2e/basicFeatures.middy.test.ts
@@ -25,7 +25,7 @@ describe(`Logger E2E tests, basic functionalities middy usage`, () => {
   const testStack = new TestStack({
     stackNameProps: {
       stackNamePrefix: RESOURCE_NAME_PREFIX,
-      testName: 'AllFeatures-Decorator',
+      testName: 'Basic-Middy',
     },
   });
 
diff --git a/packages/parameters/tests/e2e/dynamoDBProvider.class.test.ts b/packages/parameters/tests/e2e/dynamoDBProvider.class.test.ts
@@ -99,7 +99,7 @@ describe(`Parameters E2E tests, dynamoDB provider`, () => {
   const testStack = new TestStack({
     stackNameProps: {
       stackNamePrefix: RESOURCE_NAME_PREFIX,
-      testName: 'DynamoDBProvider',
+      testName: 'DynamoDB',
     },
   });
 
diff --git a/packages/parameters/tests/e2e/secretsProvider.class.test.ts b/packages/parameters/tests/e2e/secretsProvider.class.test.ts
@@ -40,7 +40,7 @@ describe(`Parameters E2E tests, Secrets Manager provider`, () => {
   const testStack = new TestStack({
     stackNameProps: {
       stackNamePrefix: RESOURCE_NAME_PREFIX,
-      testName: 'SecretsProvider',
+      testName: 'Secrets',
     },
   });
 
diff --git a/packages/parameters/tests/e2e/ssmProvider.class.test.ts b/packages/parameters/tests/e2e/ssmProvider.class.test.ts
@@ -74,7 +74,7 @@ describe(`Parameters E2E tests, SSM provider`, () => {
   const testStack = new TestStack({
     stackNameProps: {
       stackNamePrefix: RESOURCE_NAME_PREFIX,
-      testName: 'AppConfig',
+      testName: 'SSM',
     },
   });
 
diff --git a/packages/parameters/tests/helpers/resources.ts b/packages/parameters/tests/helpers/resources.ts
@@ -5,6 +5,7 @@ import type {
 } from '@aws-lambda-powertools/testing-utils';
 import {
   concatenateResourceName,
+  getRuntimeKey,
   TestDynamodbTable,
   TestNodejsFunction,
 } from '@aws-lambda-powertools/testing-utils';
@@ -57,7 +58,7 @@ class TestSecureStringParameter extends Construct {
 
     const { value } = props;
 
-    const name = `/secure/${randomUUID()}`;
+    const name = `/secure/${getRuntimeKey()}/${randomUUID()}`;
 
     const secureStringCreator = new AwsCustomResource(
       testStack.stack,
@@ -202,6 +203,7 @@ class TestDynamodbTableWithItems extends TestDynamodbTable {
       policy: AwsCustomResourcePolicy.fromSdkCalls({
         resources: [this.tableArn],
       }),
+      installLatestAwsSdk: false,
     });
   }
 }
diff --git a/packages/tracer/package.json b/packages/tracer/package.json
@@ -34,7 +34,6 @@
   "devDependencies": {
     "@aws-lambda-powertools/testing-utils": "file:../testing",
     "@aws-sdk/client-dynamodb": "^3.360.0",
-    "@aws-sdk/client-sts": "^3.360.0",
     "@aws-sdk/client-xray": "^3.360.0",
     "@types/promise-retry": "^1.1.3",
     "aws-sdk": "^2.1354.0",
diff --git a/packages/tracer/tests/e2e/allFeatures.decorator.test.ts b/packages/tracer/tests/e2e/allFeatures.decorator.test.ts
@@ -7,7 +7,6 @@ import {
   TestStack,
   TestDynamodbTable,
 } from '@aws-lambda-powertools/testing-utils';
-import { XRayClient } from '@aws-sdk/client-xray';
 import { join } from 'node:path';
 import { TracerTestNodejsFunction } from '../helpers/resources';
 import {
@@ -16,7 +15,6 @@ import {
 } from '../helpers/traceAssertions';
 import {
   getFirstSubsegment,
-  getFunctionArn,
   getInvocationSubsegment,
   getTraces,
   invokeAllTestCases,
@@ -141,7 +139,6 @@ describe(`Tracer E2E tests, all features with decorator instantiation`, () => {
   );
   testTable.grantWriteData(fnCaptureResponseOff);
 
-  const xrayClient = new XRayClient({});
   const invocationCount = 3;
 
   beforeAll(async () => {
@@ -178,28 +175,23 @@ describe(`Tracer E2E tests, all features with decorator instantiation`, () => {
       const { EXPECTED_CUSTOM_ERROR_MESSAGE: expectedCustomErrorMessage } =
         commonEnvironmentVars;
 
-      const tracesWhenAllFlagsEnabled = await getTraces(
-        xrayClient,
+      /**
+       * The trace should have 4 segments:
+       * 1. Lambda Context (AWS::Lambda)
+       * 2. Lambda Function (AWS::Lambda::Function)
+       * 4. DynamoDB (AWS::DynamoDB)
+       * 4. Remote call (docs.powertools.aws.dev)
+       */
+      const tracesWhenAllFlagsEnabled = await getTraces({
         startTime,
-        await getFunctionArn(fnNameAllFlagsEnabled),
-        invocationCount,
-        4
-      );
-
-      expect(tracesWhenAllFlagsEnabled.length).toBe(invocationCount);
+        resourceName: fnNameAllFlagsEnabled,
+        expectedTracesCount: invocationCount,
+        expectedSegmentsCount: 4,
+      });
 
       // Assess
       for (let i = 0; i < invocationCount; i++) {
         const trace = tracesWhenAllFlagsEnabled[i];
-
-        /**
-         * Expect the trace to have 4 segments:
-         * 1. Lambda Context (AWS::Lambda)
-         * 2. Lambda Function (AWS::Lambda::Function)
-         * 4. DynamoDB (AWS::DynamoDB)
-         * 4. Remote call (docs.powertools.aws.dev)
-         */
-        expect(trace.Segments.length).toBe(4);
         const invocationSubsegment = getInvocationSubsegment(trace);
 
         /**
@@ -246,13 +238,12 @@ describe(`Tracer E2E tests, all features with decorator instantiation`, () => {
         EXPECTED_CUSTOM_RESPONSE_VALUE: expectedCustomResponseValue,
       } = commonEnvironmentVars;
 
-      const tracesWhenAllFlagsEnabled = await getTraces(
-        xrayClient,
+      const tracesWhenAllFlagsEnabled = await getTraces({
         startTime,
-        await getFunctionArn(fnNameAllFlagsEnabled),
-        invocationCount,
-        4
-      );
+        resourceName: fnNameAllFlagsEnabled,
+        expectedTracesCount: invocationCount,
+        expectedSegmentsCount: 4,
+      });
 
       for (let i = 0; i < invocationCount; i++) {
         const trace = tracesWhenAllFlagsEnabled[i];
@@ -291,30 +282,24 @@ describe(`Tracer E2E tests, all features with decorator instantiation`, () => {
   it(
     'should not capture error nor response when the flags are false',
     async () => {
-      const tracesWithNoCaptureErrorOrResponse = await getTraces(
-        xrayClient,
+      /**
+       * Expect the trace to have 4 segments:
+       * 1. Lambda Context (AWS::Lambda)
+       * 2. Lambda Function (AWS::Lambda::Function)
+       * 3. DynamoDB (AWS::DynamoDB)
+       * 4. Remote call (docs.powertools.aws.dev)
+       */
+      const tracesWithNoCaptureErrorOrResponse = await getTraces({
         startTime,
-        await getFunctionArn(fnNameNoCaptureErrorOrResponse),
-        invocationCount,
-        4
-      );
-
-      expect(tracesWithNoCaptureErrorOrResponse.length).toBe(invocationCount);
+        resourceName: fnNameNoCaptureErrorOrResponse,
+        expectedTracesCount: invocationCount,
+        expectedSegmentsCount: 4,
+      });
 
       // Assess
       for (let i = 0; i < invocationCount; i++) {
         const trace = tracesWithNoCaptureErrorOrResponse[i];
-
-        /**
-         * Expect the trace to have 4 segments:
-         * 1. Lambda Context (AWS::Lambda)
-         * 2. Lambda Function (AWS::Lambda::Function)
-         * 3. DynamoDB (AWS::DynamoDB)
-         * 4. Remote call (docs.powertools.aws.dev)
-         */
-        expect(trace.Segments.length).toBe(4);
         const invocationSubsegment = getInvocationSubsegment(trace);
-
         /**
          * Invocation subsegment should have a subsegment '## index.handler' (default behavior for Tracer)
          * '## index.handler' subsegment should have 3 subsegments
@@ -358,30 +343,24 @@ describe(`Tracer E2E tests, all features with decorator instantiation`, () => {
       const { EXPECTED_CUSTOM_ERROR_MESSAGE: expectedCustomErrorMessage } =
         commonEnvironmentVars;
 
-      const tracesWithCaptureResponseFalse = await getTraces(
-        xrayClient,
+      /**
+       * Expect the trace to have 4 segments:
+       * 1. Lambda Context (AWS::Lambda)
+       * 2. Lambda Function (AWS::Lambda::Function)
+       * 3. DynamoDB (AWS::DynamoDB)
+       * 4. Remote call (docs.powertools.aws.dev)
+       */
+      const tracesWithCaptureResponseFalse = await getTraces({
         startTime,
-        await getFunctionArn(fnNameCaptureResponseOff),
-        invocationCount,
-        4
-      );
-
-      expect(tracesWithCaptureResponseFalse.length).toBe(invocationCount);
+        resourceName: fnNameCaptureResponseOff,
+        expectedTracesCount: invocationCount,
+        expectedSegmentsCount: 4,
+      });
 
       // Assess
       for (let i = 0; i < invocationCount; i++) {
         const trace = tracesWithCaptureResponseFalse[i];
-
-        /**
-         * Expect the trace to have 4 segments:
-         * 1. Lambda Context (AWS::Lambda)
-         * 2. Lambda Function (AWS::Lambda::Function)
-         * 3. DynamoDB (AWS::DynamoDB)
-         * 4. Remote call (docs.powertools.aws.dev)
-         */
-        expect(trace.Segments.length).toBe(4);
         const invocationSubsegment = getInvocationSubsegment(trace);
-
         /**
          * Invocation subsegment should have a subsegment '## index.handler' (default behavior for Tracer)
          * '## index.handler' subsegment should have 3 subsegments
@@ -428,22 +407,16 @@ describe(`Tracer E2E tests, all features with decorator instantiation`, () => {
   it(
     'should not capture any custom traces when disabled',
     async () => {
-      const expectedNoOfTraces = 2;
-      const tracesWithTracerDisabled = await getTraces(
-        xrayClient,
+      const tracesWithTracerDisabled = await getTraces({
         startTime,
-        await getFunctionArn(fnNameTracerDisabled),
-        invocationCount,
-        expectedNoOfTraces
-      );
-
-      expect(tracesWithTracerDisabled.length).toBe(invocationCount);
+        resourceName: fnNameTracerDisabled,
+        expectedTracesCount: invocationCount,
+        expectedSegmentsCount: 2,
+      });
 
       // Assess
       for (let i = 0; i < invocationCount; i++) {
         const trace = tracesWithTracerDisabled[i];
-        expect(trace.Segments.length).toBe(2);
-
         /**
          * Expect no subsegment in the invocation
          */
diff --git a/packages/tracer/tests/e2e/allFeatures.manual.test.ts b/packages/tracer/tests/e2e/allFeatures.manual.test.ts
diff --git a/packages/tracer/tests/e2e/allFeatures.middy.test.ts b/packages/tracer/tests/e2e/allFeatures.middy.test.ts
diff --git a/packages/tracer/tests/e2e/asyncHandler.decorator.test.ts b/packages/tracer/tests/e2e/asyncHandler.decorator.test.ts
diff --git a/packages/tracer/tests/helpers/tracesUtils.ts b/packages/tracer/tests/helpers/tracesUtils.ts
