Skip to content

chore(tests): address integration test flakiness #1669

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

Merged
merged 8 commits into from
Sep 4, 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
58 changes: 8 additions & 50 deletions .github/workflows/run-e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 }}
7 changes: 1 addition & 6 deletions layers/src/layer-publisher-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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}`
Expand All @@ -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(
Expand Down
1 change: 0 additions & 1 deletion layers/tests/e2e/layerPublisher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ describe(`Layers E2E tests, publisher stack`, () => {
layerName: layerId,
powertoolsPackageVersion: powerToolsPackageVersion,
ssmParameterLayerArn: ssmParameterLayerName,
removeLayerVersion: true,
});
const testLayerStack = new TestStack({
stackNameProps: {
Expand Down
1 change: 0 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/logger/tests/e2e/basicFeatures.middy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe(`Parameters E2E tests, dynamoDB provider`, () => {
const testStack = new TestStack({
stackNameProps: {
stackNamePrefix: RESOURCE_NAME_PREFIX,
testName: 'DynamoDBProvider',
testName: 'DynamoDB',
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe(`Parameters E2E tests, Secrets Manager provider`, () => {
const testStack = new TestStack({
stackNameProps: {
stackNamePrefix: RESOURCE_NAME_PREFIX,
testName: 'SecretsProvider',
testName: 'Secrets',
},
});

Expand Down
2 changes: 1 addition & 1 deletion packages/parameters/tests/e2e/ssmProvider.class.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe(`Parameters E2E tests, SSM provider`, () => {
const testStack = new TestStack({
stackNameProps: {
stackNamePrefix: RESOURCE_NAME_PREFIX,
testName: 'AppConfig',
testName: 'SSM',
},
});

Expand Down
4 changes: 3 additions & 1 deletion packages/parameters/tests/helpers/resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
} from '@aws-lambda-powertools/testing-utils';
import {
concatenateResourceName,
getRuntimeKey,
TestDynamodbTable,
TestNodejsFunction,
} from '@aws-lambda-powertools/testing-utils';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -202,6 +203,7 @@ class TestDynamodbTableWithItems extends TestDynamodbTable {
policy: AwsCustomResourcePolicy.fromSdkCalls({
resources: [this.tableArn],
}),
installLatestAwsSdk: false,
});
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/tracer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
119 changes: 46 additions & 73 deletions packages/tracer/tests/e2e/allFeatures.decorator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -16,7 +15,6 @@ import {
} from '../helpers/traceAssertions';
import {
getFirstSubsegment,
getFunctionArn,
getInvocationSubsegment,
getTraces,
invokeAllTestCases,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
*/
Expand Down
Loading