Skip to content
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 job to run the sample and check for memory leak #2071

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Conversation

sirknightj
Copy link
Contributor

@sirknightj sirknightj commented Oct 25, 2024

Issue #, if available:
N/A

What was changed?

  • GitHub actions CI. Run the master and viewer samples both with valgrind to check for memory leaks. Instead of kill with signal 9, it interrupts the process (signal 2) and also checks for graceful exit.

Why was it changed?

  • Reduce the number of times I need to manually run this check. Automating this check will also give confidence to future developers that their change did not introduce a memory leak.

How was it changed?

  • Script overview:
    • Disable logs by setting log level to 7 (silent).
    • Run the master and viewer samples in the background with valgrind. The valgrind output is captured into two different files, one for each.
    • After 30 seconds, interrupt the viewer (disconnect)
    • After 30 more seconds, interrupt the master.
    • Wait for processes to exit & capture their exit status.
    • After 10 seconds, kill the master and viewer processes. Killing the processes will make them exit with non-zero exit status.
    • Check if both exited with status 0 (whether they exit gracefully on their own after interrupt).
    • Check the valgrind output files and verify that no memory leaks are detected.

What testing was done for the changes?

  • Run it on the GitHub actions runner. Take a look at the image: image

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sirknightj sirknightj marked this pull request as ready for review October 25, 2024 23:25
@sirknightj sirknightj added the CI Automated testing label Oct 25, 2024
@sirknightj
Copy link
Contributor Author

Output when I added a memory leak for verification:
image

.github/workflows/ci.yml Show resolved Hide resolved
@sirknightj sirknightj merged commit 499eca3 into develop Nov 1, 2024
26 checks passed
@sirknightj sirknightj deleted the valgrind-job branch November 1, 2024 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Automated testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants