Add support for running units tests with valgrind. #185
Conversation
Pull Request Test Coverage Report for Build 1831551729
💛 - Coveralls |
|
The Valgrind build is failing because of legitimate memory leaks in two unit tests. The changes are behaving as expected. |
There was a problem hiding this comment.
While I knew you had submitted the fix for autoconf archive, I still got stuck trying to build and run on my local system until I looked through the workflow and saw that it's downloading your not-yet-merged changes :) By following the sequence of commands in valgrind.yml I was able to successfully run "make check-valgrind" on Fedora 34, with the expected two memcheck failures.
It would definitely help to document that autoconf dependency!
Does autoconf give any way to check the serial number of the m4 file? If someone decides to try out this feature it would be good to provide some information, since it will take a while (years?) for the autoconf-archive fix to show up in distros.
Good point. It's really only needed if someone is building mptcpd from a git repository or is modifying the autoconf/automake files shipped with the mptcpd tar archive. A mptcpd tar archive created through
The Automake manual mentions that the |
ossama-othman
force-pushed
the
valgrind-check
branch
2 times, most recently
from
db87790
to
cc5ffa5
Compare
Leverage the Autoconf Archive macro AX_VALGRIND_CHECK to enable support for running the mptcpd unit tests through valgrind, e.g.: ./configure --enable-valgrind make check-valgrind
Add an explicit rule in tests/lib/Makefile.am to build `libmptcpd.la' to allow `make check-valgrind' to be performed without first running `make' (i.e. `make all').
The GitHub pull request (autoconf-archive/autoconf-archive#242) that contains the AX_VALGRIND_CHECK GNU Autoconf Archive macro recursive `make' target fixes has been merged to the master development branch, and is now part of the v2022.02.11 release. Update the URL for the patched `ax_valgrind_check.m4' file accordingly.
Instead of modifying the `configure.ac' file, issue a warning that "make check-valgrind" may not work properly. This prevents changes to the `configure.ac' caused by the `bootstrap' script from being inadvertently committed to a git repository.
ossama-othman
force-pushed
the
valgrind-check
branch
from
cc5ffa5
to
f778439
Compare
Running `wget' on the GITWEB based URL caused a HTML page to be downloaded instead of the plain text `ax_valgrind.check.m4' file. Use the CGIT based URL instead.
|
I should probably bump the copyright year on the modified files. |
| echo Patched AX_VALGRIND_CHECK GNU Autoconf Archive macro not found. | ||
| echo -n "Attempting to download patched ax_valgrind_check.m4... " | ||
| cd m4 | ||
| wget --quiet --timeout 10 http://git.savannah.gnu.org/cgit/autoconf-archive.git/plain/m4/ax_valgrind_check.m4 |
There was a problem hiding this comment.
Perhaps it would be better to make the download optional and switched off by default.
Pull Request Test Coverage Report for Build 1831288835Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 1550974842Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Leverage the Autoconf Archive macro
AX_VALGRIND_CHECKto enable support for running the mptcpd unit tests through valgrind, e.g.:./configure --enable-valgrind make check-valgrind