GNOME Bugzilla – Bug 732144
videobox: Add unit tests
Last modified: 2014-06-28 08:28:25 UTC
Created attachment 279082 [details] [review] Unit test for videobox element. Initial version covering format-conversion tests videobox element doesn't have unit tests in tests/check/elements Attaching initial version of videobox unit test file covering format conversions test cases.
Review of attachment 279082 [details] [review]: Thanks for the patch, good work :) Just some minor comments ::: tests/check/elements/videobox.c @@ +151,3 @@ + state_ret = gst_element_set_state (ctx.pipeline, GST_STATE_PAUSED); + fail_unless (state_ret != GST_STATE_CHANGE_FAILURE, + "couldn't set pipeline to PAUSED state"); This is racy... very unlikely but it could happen that you shutdown your element already before it actually negotiates :) Try setting num-buffers=1 on videotestsrc and then wait for the EOS message (or an ERROR message) on the bus.
Created attachment 279103 [details] [review] New patch : Handled sync Thanks for the review. New patch attached.
Looks good, but why do you first go to PAUSED and then READY? Just directly to READY should work too
Review of attachment 279103 [details] [review]: And some nitpicking ::: tests/check/Makefile.am @@ +375,3 @@ $(check_taglib) \ $(check_udp) \ + $(check_videobox) \ Indention is broken here ::: tests/check/elements/videobox.c @@ +44,3 @@ +{ + gchar in_caps[MAX_CAPS_LEN]; + gchar out_caps[MAX_CAPS_LEN]; You can also make them const gchar *in_caps; etc. No need to fiddle with the length here @@ +52,3 @@ + * Update this table as and when the conversion is supported(or unsupported) in videobox + */ +static FormatConversion ConversionTable[] = { We name variables lowercase and separated with _ usually, so conversion_table. Also make this table const :) @@ +170,3 @@ + gst_object_unref (bus); + + conversions_test_size = sizeof (ConversionTable) / sizeof (FormatConversion); G_N_ELEMENTS(ConversionTable) @@ +224,3 @@ + suite_add_tcase (s, tc_chain); + tcase_add_test (tc_chain, test_caps_transform); + /* FIXME: Add relevant test cases */ FIXME comment can go away
Created attachment 279166 [details] [review] Incorporated review suggestions New patch after including review comments. I deliberately moved the pipeline to all states while testing. I can get rid of paused state. My editor probably expanded the tabs and broke the indentation. It looks good to me now. Please have a look. Thanks.
commit e4f0133cb194b186e1ddbc21c77ce3ea451bfd9d Author: Ravi Kiran K N <ravi.kiran@samsung.com> Date: Tue Jun 24 09:14:40 2014 +0530 videobox: Add unit test https://bugzilla.gnome.org/show_bug.cgi?id=732144