Skip to content

fix: GitHub action CI #130

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
Apr 30, 2025
Merged

fix: GitHub action CI #130

merged 8 commits into from
Apr 30, 2025

Conversation

maxday
Copy link
Contributor

@maxday maxday commented Apr 30, 2025

Description of changes:

  • Add locally generated codebuild files to .gitignore
  • Bump nodejs version to 18 (16 being deprecated on AWS Lambda)
  • Move out from makefile and codebuild to run unit test, using a simple matrix job + Dockerfile
  • Refactor integration test runs into a single matrix job
  • Comment out 4 tests which were failing (see follow up actions)
  • Remove ubuntu 18.04 as node18 needs a version of libc which is imcompatible with ubuntu18.04 failing with:
The following packages have unmet dependencies:
 nodejs : Depends: libc6 (>= 2.28) but 2.27-3ubuntu1.6 is to be installed
E: Unable to correct problems, you have held broken packages.

Follow-up:

  • CI needs to be upgraded to we can test nodejs.22x as well. I'm not doing this in this PR because we need to simplify a lot how the testing is done.
  • Investigate why 4 tests were failing, it seems related to the node version we're running them against (passing on node18). We might need 3 branches (see Investigate test/unit/UserFunctionTest.js failures #131)

Target (OCI, Managed Runtime, both):
both

Checklist

  • I have run npm install to generate the package-lock.json correctly and included it in the PR.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -0,0 +1,7 @@
FROM node:20
RUN apt-get update
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have simplified into one single Dockerfile and passing the NODEJS_MAJOR as build arg, but I feel it's short enough to have it 3 times, allowing us to have potential drift in Dockerfile later

@maxday maxday marked this pull request as ready for review April 30, 2025 10:13
@godcrampy godcrampy self-requested a review April 30, 2025 13:52
@maxday maxday merged commit ad1d551 into main Apr 30, 2025
9 checks passed
@maxday maxday mentioned this pull request Apr 30, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants