GNOME Bugzilla – Bug 723492
gst-plugins-base: Do not build check tests for disabled plugins
Last modified: 2014-02-11 20:01:35 UTC
If I configure gst-plugins-base like so: ./configure --disable-audioconvert and build: make -j10 and then try to run the check tests: make -j10 -C tests/check check then I will see that the following test fails: FAIL: elements/audioconvert This is surprising since I actively disabled audioconvert. The flag disabled building the plugin but not the check tests. There are additional tests that fail due to the audioconvert plugin is not being built: FAIL: elements/encodebin FAIL: elements/audiorate FAIL: elements/adder FAIL: pipelines/vorbisdec FAIL: pipelines/simple-launch-lines FAIL: pipelines/vorbisenc FAIL: pipelines/oggmux FAIL: elements/audioresample These tests could potentially be rewritten in the future to not depend on audioconvert, so these are left unchanged. The attached patch only disables the each plugin's own checks when the plugin is disabled, not just for audioconvert, but for all plugins that can be disabled.
Created attachment 267876 [details] [review] Proposed patch that disables the check tests for disabled plugins.
Created attachment 267897 [details] [review] Proposed patch that disables the check tests for disabled plugins. Fixed some accidental whitespace damage in the original patch.
Review of attachment 267897 [details] [review]: ::: tests/check/Makefile.am @@ +64,3 @@ + +if USE_PLUGIN_PLAYBACK +check_playback = elements/decodebin elements/playbin streamsynchronizer and playbin-complex is in the playback plugin too @@ +203,3 @@ pipelines/simple-launch-lines \ pipelines/basetime \ pipelines/capsfilter-renegotiation \ From the above many will probably fail too @@ +209,3 @@ + $(check_audioresample) \ + elements/multifdsink \ + elements/multisocketsink \ What about these two?
Created attachment 268111 [details] [review] Proposed patch that disables the check tests for disabled plugins. Fixed review comment about playback and tcp plugins.
Review of attachment 268111 [details] [review]: Looks better but still... :) ::: tests/check/Makefile.am @@ +182,1 @@ elements/audiotestsrc \ What about audiotestsrc? @@ +209,3 @@ pipelines/simple-launch-lines \ pipelines/basetime \ pipelines/capsfilter-renegotiation \ Are the pipelines tests working properly if everything is disabled?
> Looks better but still... :) yeah, yeah... I'll go over it one more time. hopefully I'll learn from my mistakes and be more careful when submitting the one for -good again. > ::: tests/check/Makefile.am > @@ +182,1 @@ > elements/audiotestsrc \ > > What about audiotestsrc? Mmm, --disable-audiotestsrc is there so this one makes sense. I'll fix it. > @@ +209,3 @@ > pipelines/simple-launch-lines \ > pipelines/basetime \ > pipelines/capsfilter-renegotiation \ > > Are the pipelines tests working properly if everything is disabled? These ones are more doubtful. There a are no obivous --disable-basetime (which depends on audiotestsrc and fakesink) or --disable-simple-launch-lines (which depends on alsasink, audioconvert, audioresample, audiotestsrc, capsfilter, fakesink, fakesrc, libvisual_lv_scope, tee, videoconvert, videoscale, videotestsrc and xvimagesink). I spoke briefly with __tim on the subject and, as I understood him, the best approach is add ifdef-guards for complete check tests that obviously are related to a specific plugin. For tests like basetime or simple-launch-lines where the check test itself can not be obviously connected to any specific --disable-flag then it might be better with a run-time approach. This would apply for any individual testcase where an element is created that is not in the plugin being tested. To do this in run-time I think something along the lines of gst_element_factory_find() could be queried (most likely through some kind of convenience function, perhaps a tcase_add_with_dependencies()) and the tests skipped if any of those elements are missing. Since this may require new convenience functions I believe that it would make more sense to do those in a separate patch, and only go for disabling the check tests that can obviously be associated with a particular plugin in this patch.
Created attachment 268366 [details] [review] Proposed patch that disables the check tests for disabled plugins. Fixed audiotestsrc and resorted the check_PROGRAMS dependencies.
commit 09644d0e4a8bab38c045306d509b6f27ba2567a8 Author: Sebastian Rasmussen <sebras@hotmail.com> Date: Sun Feb 2 23:59:36 2014 +0100 tests: Don't build disabled plugins' check tests Fixes https://bugzilla.gnome.org/show_bug.cgi?id=723492