-
Notifications
You must be signed in to change notification settings - Fork 238
Enabling Local Testing with RIE and Improving Tests Coverability #560
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
@@ -74,5 +78,5 @@ TARGETS | |||
dev Run all development tests after a change. | |||
pr Perform all checks before submitting a Pull Request. | |||
test Run the Unit tests. | |||
|
|||
test-rie Build and test RIC locally with Lambda Runtime Interface Emulator. (Requires building the project first) |
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!
Thanks @AntoniaSzecsi ! |
Can you change this as well so we won't go lower https://github.com/aws/aws-lambda-java-libs/blob/main/aws-lambda-java-runtime-interface-client/pom.xml#L245 |
@@ -0,0 +1,8 @@ | |||
FROM public.ecr.aws/lambda/java:11 |
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.
let's use 21?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #560 +/- ##
============================================
+ Coverage 58.26% 64.74% +6.47%
+ Complexity 222 194 -28
============================================
Files 46 33 -13
Lines 1155 936 -219
Branches 149 132 -17
============================================
- Hits 673 606 -67
+ Misses 428 281 -147
+ Partials 54 49 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
I think we should avoid writing tests just to make the coverage analysis pass. Unit tests should be meaningful and help document how our application actually works.
My suggestion is to evaluate whether what you are testing is truly meaningful, and if not, adjust the coverage tool to exclude it. As a general guideline:
-
Exceptions and simple POJOs are usually excluded from coverage in most projects I’ve seen. Exceptions can be made for classes that exhibit significant behavior.
-
In typed languages, you generally don’t need tests for features already enforced by the compiler (e.g., the Resource interface).
-
Constants and environment variables should only be tested if they are critical to a specific business process, and in that case, the test should focus on where they are used.
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.
LGTM 🚀
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.
Great work on the readme and the test script!
<configuration> | ||
<excludes> | ||
<!-- Exclude simple exceptions (compiler enforced) --> | ||
<exclude>**/*Exception.class</exclude> | ||
<!-- Exclude interfaces (compiler enforced) --> | ||
<exclude>**/Resource.class</exclude> | ||
<!-- Exclude simple DTOs/POJOs (data containers) --> | ||
<exclude>**/dto/*.class</exclude> | ||
<!-- Exclude constants classes (build-time values) --> | ||
<exclude>**/ReservedRuntimeEnvironmentVariables.class</exclude> | ||
<exclude>**/RapidErrorType.class</exclude> | ||
<!-- Exclude enum-like data holders --> | ||
<exclude>**/FrameType.class</exclude> | ||
<exclude>**/StructuredLogMessage.class</exclude> | ||
<!-- Exclude main entry point (runtime-only) --> | ||
<exclude>**/AWSLambda.class</exclude> | ||
</excludes> | ||
</configuration> |
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!
Issue #, if available:
Description of changes:
Target (OCI, Managed Runtime, both):
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
This PR enhances the aws-lambda-java-runtime-interface-client with local testing capabilities and significantly improves test coverage.
Summary of changes:
Dockerfile.rie
for containerized local testingtest-rie.sh
for automated testing workflowEchoHandler
for testing purposes