Skip to content

Update axios to 1.8.2 in e2e tests #624

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

Closed
wants to merge 1 commit into from

Conversation

rockdaboot
Copy link
Contributor

@rockdaboot rockdaboot self-assigned this Mar 13, 2025
@github-actions github-actions bot added the aws-λ-extension AWS Lambda Extension label Mar 13, 2025
@dmathieu
Copy link
Member

This goes a major bump. Are we sure there aren't any untested breaking changes?
cc @trentm

@trentm
Copy link
Member

trentm commented Mar 13, 2025

I guess I don't have perms on this repo to view https://github.com/elastic/apm-aws-lambda/security/dependabot/43

@trentm
Copy link
Member

trentm commented Mar 13, 2025

These e2e-testing bits are, I gather, very out of date. It looks like the e2e-testing bits aren't run regularly.

  • this axios update is fine, the small "app.js" in this package isn't even using the axios dep, so it could be removed.

This sam-testing-nodejs dir was adapted from a small aws-sam-cli test package that now lives here: https://github.com/aws/aws-sam-cli/tree/develop/samcli/lib/init/templates/cookiecutter-aws-sam-hello-nodejs/%7B%7Bcookiecutter.project_name%7D%7D/hello-world
That includes a dep on axios, but has any code using axios commented out.
The adapted copy here did not copy over the unit tests, so:

  • the devDependencies could all be removed as well

  • As well, the elastic-apm-node dependency is on an old major, the current supported versions are 4.x.

@rockdaboot Do you have a preference here? We could (a) take this PR as is; (b) use this or a separate PR to make these other updates. I'm happy to do the quick updates for (b).

@rockdaboot
Copy link
Contributor Author

rockdaboot commented Mar 13, 2025

Do you have a preference here? We could (a) take this PR as is; (b) use this or a separate PR to make these other updates. I'm happy to do the quick updates for (b).

@trentm It looks like (b) would be the proper thing to do. I'd would be great if you can do it!

trentm added a commit to trentm/apm-aws-lambda that referenced this pull request Mar 13, 2025
This removes deps not being used by the 'sam-testing-nodejs' package
used in e2e-testing. It bumps elastic-apm-node to its current major
version.

This also makes some attempts to get e2e-testing working again, but
ultimately I did not get far. e2e-testing looks to not have been
updated for changes in how the apm-lambda-extension is now built.

Obsoletes: elastic#624
@trentm
Copy link
Member

trentm commented Mar 13, 2025

It looks like (b) would be the proper thing to do. I'd would be great if you can do it!

#626

kruskall pushed a commit that referenced this pull request Mar 14, 2025
This removes deps not being used by the 'sam-testing-nodejs' package
used in e2e-testing. It bumps elastic-apm-node to its current major
version.

This also makes some attempts to get e2e-testing working again, but
ultimately I did not get far. e2e-testing looks to not have been
updated for changes in how the apm-lambda-extension is now built.

Obsoletes: #624
@rockdaboot
Copy link
Contributor Author

Closing in favor of #626

@rockdaboot rockdaboot closed this Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-λ-extension AWS Lambda Extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants