[Bug 747100] tests: filesink: add test for GstFileSink render_list implemention

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=747100

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #300655|none                        |reviewed
             status|                            |

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 300655
  --> https://bugzilla.gnome.org/attachment.cgi?id=300655
filesink: test_seeking test is updated to verify the implementation of virtual
method render_list

Thanks a lot for the patch. Better unit test coverage is always welcome.

Two small nitpicks:

>Subject: [PATCH] filesink: test_seeking test is updated to verify the
> implementation of virtual method render_list

This 'summary line' is quite long, try to make it as short and concise as
possible, e.g. "tests: filesink: add check for render_list virtual method" or
similar.

>GstFileSink implements render_list vitual method to render list of buffers. There is no method present till now in filesink testsuit to check the functionality of this virtual method implementation by GstFileSink.
>Updating the test_seeking testcase to check render_list method implementation where it push the buffer_list to the pad and verify the position to make sure it is successfully written to file.

It's great that you took the time to write a detailed commit message, but this
might be a bit too much detail really. You usualy don't have to justify why you
add a new unit test, it's self-evident why it's a good thing. Please wrap the
commit message lines (not the summary line) at ca. ~80 characters or so.

About the code: it looks good, but it would be great if we could also check the
actual data written out if it's not too much extra work (there seems to be a
macro for that already).

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.