Skip to content

Fix(build): e2e workflow failing #1047

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 5 commits into from
Aug 10, 2022
Merged

Fix(build): e2e workflow failing #1047

merged 5 commits into from
Aug 10, 2022

Conversation

dreamorosi
Copy link
Contributor

Description of your changes

As described in #1046 there is a small bug in the newly introduced layer job that is now part (#826) of the e2e tests.

The issue was caused by the fact that the e2e tests for this part of the codebase required code in the packages/commons folder. The dependencies for that packages were not being installed causing the run to fail.

After fixing the issue I then found out two more issues:

  • The workflow uses the matrix strategy to run the e2e tests on multiple NodeJS versions. There was a resource in the tested stack (an SSM Param) for which the name was not unique making 2/3 versions of the matrix fail automatically. I have added a suffix with the NodeJS version being tested to make the name unique.
  • The workflow deploys a Lambda function that is used to test the correctness of the Lambda Layer being tested. That function imports the Powertools utilities. Since the utils are expected to be in the layer, they are not installed (npm i) as part of the setup process. This was however causing the esbuild process to fail because they were not marked as external. Marking a dependency as external tells CDK & esbuild to not bundle said dependency since it's expected to be present in the Lambda environment.

This PR aims at fixing all three issues mentioned above.

How to verify this change

See this cool badge for the e2e test results for this branch:
run-e2e-tests

Related issues, RFCs

Issue number: #1046

PR status

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

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • The PR title follows the conventional commit semantics

Breaking change checklist

N/A


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

@dreamorosi dreamorosi added automation This item relates to automation fix labels Aug 10, 2022
@dreamorosi dreamorosi added this to the Lambda layer milestone Aug 10, 2022
@dreamorosi dreamorosi self-assigned this Aug 10, 2022
Copy link
Contributor

@flochaz flochaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YOU ROCK!

uses: actions/checkout@v3
- name: "Use NodeJS 14"
- name: Use NodeJS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the house keeping!

path: "./node_modules"
# Use the combo between node version, name, and SHA-256 hash of the lock file as cache key so that
# if one of them changes the cache is invalidated/discarded
key: ${{ matrix.version }}-cache-utils-node-modules-${{ hashFiles('./package-lock.json') }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@dreamorosi dreamorosi linked an issue Aug 10, 2022 that may be closed by this pull request
@dreamorosi dreamorosi merged commit a0f1db2 into main Aug 10, 2022
@dreamorosi dreamorosi deleted the fix/build/e2e_workflow branch August 10, 2022 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation This item relates to automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: e2e tests failing
3 participants