-
Notifications
You must be signed in to change notification settings - Fork 153
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
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
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:
npm i
) as part of the setup process. This was however causing theesbuild
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:

Related issues, RFCs
Issue number: #1046
PR status
Is this ready for review?: YES
Is it a breaking change?: NO
Checklist
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.