GNOME Bugzilla – Bug 746465
validate: add valgrind support
Last modified: 2015-03-20 09:40:43 UTC
Adding a --valgrind option to gst-validate-launcher to track memory corruption and leaks. That's also the first step for automatically checking for leak regressions once we'll have a proper suppression file.
Created attachment 299837 [details] [review] launcher: add valgrind support Add a --valgrind option to gst-validate-launcher to run the tests inside Valgrind and tune GLib's memory allocator accordingly. Fix
Review of attachment 299837 [details] [review]: Looks good but I wonder if you should not make the timeout and hard timeout longer in that case (probably * 5 or something) ? Would be nice if we could make test fails if we detect definitely lost memory (probably redirecting valgrind logs to a specific file and parse it). That could be added later.
Created attachment 299847 [details] [review] launcher: add valgrind support Add a --valgrind option to gst-validate-launcher to run the tests inside Valgrind and tune GLib's memory allocator accordingly. Fix bgo#746465
Created attachment 299850 [details] [review] launcher: add valgrind support Add a --valgrind option to gst-validate-launcher to run the tests inside Valgrind and tune GLib's memory allocator accordingly. Fix
Created attachment 299851 [details] [review] launcher: try using gst.supp as valgrind suppressions file
Review of attachment 299851 [details] [review]: ::: validate/launcher/baseclasses.py @@ +565,3 @@ + def get_valgrind_suppressions(self): + # Are we running from sources? + p = os.path.join('common', 'gst.supp') You should base yourself on __file__ rather than $PWD @@ +569,3 @@ + return p + + # TODO: do we want to install gst.supp and try using it from $prefix? We could install it into share/gstreamer-1.0/ or something like that maybe?
Created attachment 299854 [details] [review] launcher: add valgrind support Add a --valgrind option to gst-validate-launcher to run the tests inside Valgrind and tune GLib's memory allocator accordingly. Fix
Created attachment 299855 [details] [review] validate: install gst.supp Will be used when running tests inside Valgrind.
Created attachment 299856 [details] [review] launcher: try using gst.supp as valgrind suppressions file
Review of attachment 299856 [details] [review]: ::: validate/launcher/Makefile.am @@ +17,3 @@ +config.py: Makefile + $(AM_V_GEN) { \ + echo "DATADIR = '${datadir}/gstreamer-$(GST_API_VERSION)/validate-suppression'"; \ What is that for? (I can not see it used anywhere) ::: validate/launcher/baseclasses.py @@ +573,3 @@ + # Look in system data dirs + for datadir in GLib.get_system_data_dirs(): + if os.path.exists(p): We should use gstreamer-1.0/validate/gst.sup and gstreamer1.0/validate/scenarios/ I think... no big deal I guess.
(In reply to Thibault Saunier from comment #10) > Review of attachment 299856 [details] [review] [review]: > > ::: validate/launcher/Makefile.am > @@ +17,3 @@ > +config.py: Makefile > + $(AM_V_GEN) { \ > + echo "DATADIR = > '${datadir}/gstreamer-$(GST_API_VERSION)/validate-suppression'"; \ > > What is that for? (I can not see it used anywhere) Oh sorry this wasn't meant to be commited. > ::: validate/launcher/baseclasses.py > @@ +573,3 @@ > + # Look in system data dirs > + for datadir in GLib.get_system_data_dirs(): > + if os.path.exists(p): > > We should use gstreamer-1.0/validate/gst.sup and > gstreamer1.0/validate/scenarios/ I think... no big deal I guess. I can move those yeah.
Created attachment 299925 [details] [review] validate: move scenarios to validate/scenarios/
Created attachment 299926 [details] [review] launcher: add valgrind support Add a --valgrind option to gst-validate-launcher to run the tests inside Valgrind and tune GLib's memory allocator accordingly. Fix
Created attachment 299927 [details] [review] validate: install gst.supp Will be used when running tests inside Valgrind.
Created attachment 299928 [details] [review] launcher: try using gst.supp as valgrind suppressions file
Review of attachment 299925 [details] [review]: Looks good, I am just wondering if it will break backward compatibility but I think it won't actually as scenarios are bundled with -validate itself.
Review of attachment 299926 [details] [review]: Looks good
Review of attachment 299927 [details] [review]: Looks good.
Review of attachment 299928 [details] [review]: Looks good.
commit cb8348c7b18485761d7022c2200c82bf001b4e5f Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk> Date: Thu Mar 19 17:22:26 2015 +0100 launcher: try using gst.supp as valgrind suppressions file https://bugzilla.gnome.org/show_bug.cgi?id=746465 commit 3024b3682fb2ddd074431103295e6f534b7d2ebf Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk> Date: Thu Mar 19 17:44:19 2015 +0100 validate: install gst.supp Will be used when running tests inside Valgrind. https://bugzilla.gnome.org/show_bug.cgi?id=746465 commit 271f9f3c8edeedb21fa5d1288b11e98b6d44d572 Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk> Date: Thu Mar 19 16:06:54 2015 +0100 launcher: add valgrind support Add a --valgrind option to gst-validate-launcher to run the tests inside Valgrind and tune GLib's memory allocator accordingly. Fix https://bugzilla.gnome.org/show_bug.cgi?id=746465 commit 2778e501c4c05bb69227340bbc43e7995c8082b2 Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk> Date: Fri Mar 20 10:06:35 2015 +0100 validate: move scenarios to validate/scenarios/ https://bugzilla.gnome.org/show_bug.cgi?id=746465