Merged
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #3988 +/- ##
==========================================
+ Coverage 79.53% 79.55% +0.01%
==========================================
Files 140 140
Lines 15594 15594
Branches 2647 2647
==========================================
+ Hits 12403 12406 +3
- Misses 2351 2355 +4
+ Partials 840 833 -7 |
ffe9693 to
d12abf5
Compare
2cde31a to
a89b3c4
Compare
antonpirker
commented
Feb 24, 2025
Because we changed our AWS Lambda test suite to not create real Lambda functions but rather run a Lambda environment locally, we do not need all the code necessary for handling the authentication to AWS in the test suite. This PR removes all the AWS auth related code and als moves the AWS Lambda testsuite again into the "Cloud" group of test suites, because it is a "normal" test suite again. The PR that includes the change to have a local Lambda environment is here: #3988
sentrivana
reviewed
Feb 25, 2025
Contributor
sentrivana
left a comment
There was a problem hiding this comment.
Overall looking great. I tried comparing the old tests to the new ones and it seems to me like we might not be testing the following anymore:
- sampling context being correct (old test name was
test_traces_sampler_gets_correct_values_in_sampling_context) - different behavior if perf is enabled/disabled (tests ending in
..._performance_enabled/disabled)
Maybe this is included in the new tests, it's just hard to diff 😇
df14176 to
2e75f79
Compare
sentrivana
approved these changes
Mar 12, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test Sentry AWS Lambda integration locally instead of creating actual Lambda function in AWS:
There is also a follow-up PR that removes obsolete code handling AWS authentication data: #4076 (This PR will also fix the one failing test)
Fixes #2795