-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add valgrind in CI #160
Add valgrind in CI #160
Conversation
CI failure caused by issue eclipse-uprotocol/up-conan-recipes#9 |
95186bf
to
62da57d
Compare
@BishrutSubedi - It looks like the change in 38cb0ba worked. The tests that failed are sensitive to memory allocation throughput, which is reduced when valgrind is running. I think it might be safe to disable those tests when running valgrind. What do you think? |
Sounds good to me |
7b4de0e
to
9ceaf2e
Compare
failing unit tests, and runs remaining unit test with valgrind
9ceaf2e
to
1681997
Compare
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've only got a few questions and a couple of small changes listed. Otherwise this looks ready to go. Thanks for getting valgrind added to the CI process!
I've been poking at this a little more and I'm not sure how to interpret results out of it. For example, I have a run where I added an intentional memory leak. The valgrind output from the log looks like this:
When I run one of the test binaries under valgrind directly, I get this instead:
In both cases, I modified the process to use the Debug build type just to see if adding debug symbols helped. |
With the current setup, the valgrind logs will have the names of test that failed. For more detailed breakdown of lines that failed, I run the failed test locally directly under valgrind. If build with build type debug, we get the exact lines that caused failure. In release, we get only the top level function call of where it failed, but the subsequent lines are hidden. We can think this as summary of what failed, and investigate locally to see the result. One way to get this detail result in CI would be to collect tests, under test dir, and run it directly with valgrind and concatenate the results |
97d39b5
to
f87cc5a
Compare
fc79815
to
8a0d03c
Compare
77dbda6
to
619decf
Compare
This PR adds dynamic analysis check i.e. valgrind to run in all unit test in CI
It also adds a text file to exclude any test to run under valgrind and it's tools
To skip test from running, add it in format
<test binary name>.<testsuite name>.<test name>