After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723953 - mpegts: Unit test for library RFC
mpegts: Unit test for library RFC
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other Linux
: Normal enhancement
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-02-09 11:18 UTC by Jesper Larsen
Modified: 2014-02-24 15:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: Add test for mpegts library (9.99 KB, patch)
2014-02-09 11:18 UTC, Jesper Larsen
needs-work Details | Review
tests: Add test for mpegts library (12.80 KB, patch)
2014-02-22 22:18 UTC, Jesper Larsen
committed Details | Review
mpegts: network_name: Check converted length (1.02 KB, patch)
2014-02-24 13:11 UTC, Jesper Larsen
committed Details | Review

Description Jesper Larsen 2014-02-09 11:18:37 UTC
Created attachment 268564 [details] [review]
tests: Add test for mpegts library

First draft for a unit test to the mpegts library

For starters only PAT, PMT, and NIT are tested, as these are the only tables we have constructors for.

Ideas on how we can test the integrity of descriptors are welcome.
Problem is that i.e the network_name descriptor have practically endless possibilities.

Likewise there is endless possibilities for combinations of sections and descriptors.
Comment 1 Edward Hervey 2014-02-20 12:26:22 UTC
Review of attachment 268564 [details] [review]:

It goes in the right direction.

Some stuff missing:
* More descriptor checks (starting by the ones you use)
* Testing expected failures

::: tests/check/libs/mpegts.c
@@ +91,3 @@
+    if (data[i] != pat_data_check[i]) {
+      gchar *msg;
+      msg = g_strdup_printf ("0x%X != 0x%X in byte %d of PAT section",

You don't need a temporary string for this, you can use varargs with fail
Comment 2 Jesper Larsen 2014-02-22 22:18:54 UTC
Created attachment 270015 [details] [review]
tests: Add test for mpegts library

Updates to the patch.

Introduced tests for descriptors. Does only contain descriptors that can be created through the lib at the moment.

I was contemplating on creating tests for the conversion of UTF-8 strings to different character maps, but the dvb_text_from_utf8 is private as it is now. Furthermore, I guess that the conversion is not guaranteed to be the same across different platforms?
Comment 3 Edward Hervey 2014-02-24 09:55:58 UTC
Hmm... I'm getting quite this failure. You sure the checks are correct ?

Running suite(s): MPEG Transport Stream helper library
80%: Checks: 5, Failures: 1, Errors: 0
libs/mpegts.c:347:F:general:test_mpegts_dvb_descriptors:0: Expected g_critical, got nothing
Comment 4 Jesper Larsen 2014-02-24 13:11:42 UTC
Created attachment 270124 [details] [review]
mpegts: network_name: Check converted length

Oh forgot about that, sorry. It is actually a small bug, which is fixed in this patch.
Comment 5 Edward Hervey 2014-02-24 15:03:11 UTC
Commited, thx