-
Notifications
You must be signed in to change notification settings - Fork 181
Add fault-injection unit tests (coverage part 2/3) #766
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
55b4a78 to
e1ec1db
Compare
ecd08bc to
ffefb0f
Compare
cf14a57 to
56ba274
Compare
ce10329 to
599c3c8
Compare
Blast545
left a comment
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
5940d72 to
8b9dd40
Compare
b98697c to
a7af61a
Compare
|
I checked these tests for memory leaks. I discovered two additional ones when running |
8b9dd40 to
422dd65
Compare
|
@brawner could it be a fault injection-induced leak in |
422dd65 to
337f9ee
Compare
a7af61a to
81d411c
Compare
68fb9cc to
37cac47
Compare
81d411c to
0446196
Compare
|
@hidmic Yes I believe that's correct. It might be harder to address since we need those injected faults for coverage. |
hidmic
left a comment
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 !
| "string_array_with_quoted_number.yaml" | ||
| }; | ||
|
|
||
| for (auto & filename : filenames) { |
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.
@brawner nit: can be made const?
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 const is already deduced by auto since filenames is const.
Signed-off-by: Stephen Brawner <[email protected]>
Signed-off-by: Stephen Brawner <[email protected]>
Signed-off-by: Stephen Brawner <[email protected]>
Signed-off-by: Stephen Brawner <[email protected]>
0446196 to
8ca6f42
Compare
|
Rebased onto master after merging #771 |
* Fault injection tests for rcl_yaml Signed-off-by: Stephen Brawner <[email protected]> * PR Feedback Signed-off-by: Stephen Brawner <[email protected]> * Pause fault injection Signed-off-by: Stephen Brawner <[email protected]> * variant_copy Signed-off-by: Stephen Brawner <[email protected]>
* Fault injection tests for rcl_yaml Signed-off-by: Stephen Brawner <[email protected]> * PR Feedback Signed-off-by: Stephen Brawner <[email protected]> * Pause fault injection Signed-off-by: Stephen Brawner <[email protected]> * variant_copy Signed-off-by: Stephen Brawner <[email protected]>
This adds unit tests to rcl_yaml_param_parser to increase coverage to 95.2%. Since the PR is so long, I'm leaving it in draft for now and will plan to refactor it into a couple of parts for easier review.
Depends on #765
Signed-off-by: Stephen Brawner [email protected]