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 752027 - dashdemux: tests: improve unit test code coverage
dashdemux: tests: improve unit test code coverage
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-06 16:37 UTC by Florin Apostol
Modified: 2015-08-16 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch 1/5 (23.92 KB, patch)
2015-07-06 16:38 UTC, Florin Apostol
committed Details | Review
patch 2/5 (2.91 KB, patch)
2015-07-06 16:38 UTC, Florin Apostol
committed Details | Review
patch 3/5 (1.05 KB, patch)
2015-07-06 16:38 UTC, Florin Apostol
committed Details | Review
patch 4/5 (46.25 KB, patch)
2015-07-06 16:39 UTC, Florin Apostol
committed Details | Review
patch 5/5 (57.56 KB, patch)
2015-07-06 16:39 UTC, Florin Apostol
none Details | Review
replaced patch 5/5 (57.88 KB, patch)
2015-07-07 16:05 UTC, Florin Apostol
none Details | Review
replaced patch 5/5 (58.83 KB, patch)
2015-07-08 19:41 UTC, Florin Apostol
committed Details | Review

Description Florin Apostol 2015-07-06 16:37:50 UTC
Improve the code coverage of dashdemux unit tests: achieve 100% function coverage.
Comment 1 Florin Apostol 2015-07-06 16:38:16 UTC
Created attachment 306935 [details] [review]
patch 1/5
Comment 2 Florin Apostol 2015-07-06 16:38:33 UTC
Created attachment 306936 [details] [review]
patch 2/5
Comment 3 Florin Apostol 2015-07-06 16:38:52 UTC
Created attachment 306937 [details] [review]
patch 3/5
Comment 4 Florin Apostol 2015-07-06 16:39:10 UTC
Created attachment 306938 [details] [review]
patch 4/5
Comment 5 Florin Apostol 2015-07-06 16:39:56 UTC
Created attachment 306939 [details] [review]
patch 5/5

tested all functions. Code coverage is:

  lines......: 83.8% (1941 of 2316 lines)
  functions..: 100.0% (141 of 141 functions)
Comment 6 Sebastian Dröge (slomo) 2015-07-07 11:16:17 UTC
Comment on attachment 306939 [details] [review]
patch 5/5

98%: Checks: 85, Failures: 1, Errors: 0
elements/dash_mpd.c:3118:F:complexMPD:dash_mpdparser_segments:0: 'flow' (-3) is not equal to 'GST_FLOW_OK' (0)
Makefile:3867: recipe for target 'elements/dash_mpd.check' failed
Comment 7 Florin Apostol 2015-07-07 16:05:51 UTC
Created attachment 307019 [details] [review]
replaced patch 5/5

The problem is caused by the changes introduced in bug https://bugzilla.gnome.org/show_bug.cgi?id=751850. I've raised bug https://bugzilla.gnome.org/show_bug.cgi?id=752085 to explain why.

Please resolve bug 752085 and then these tests will not have any problem.

I've updated patch 5/5 a little bit: I've updated some comments to better explain the test scenario. See the new patch attached.
Comment 8 Sebastian Dröge (slomo) 2015-07-07 16:39:50 UTC
Comment on attachment 307019 [details] [review]
replaced patch 5/5

Looks good, will merge in a bit with other changes.
Comment 9 Sebastian Dröge (slomo) 2015-07-07 16:53:46 UTC
Comment on attachment 307019 [details] [review]
replaced patch 5/5

This fails now with latest GIT:
98%: Checks: 85, Failures: 1, Errors: 0
elements/dash_mpd.c:3119:F:complexMPD:dash_mpdparser_segments:0: 'hasNextSegment' (0) is not equal to '1' (1)
Makefile:3867: recipe for target 'elements/dash_mpd.check' failed


The test in question checks if with segment_index=0 and segments_count=1, there has_next_segment() returns TRUE. It assumes it should, but it doesn't. Which is AFAIU correct: if you have segment_index=0 and segments_count=1, then there is no next segment. Segment indices start at 0 and go up to segment_count-1.

Without the change that broke your test, we won't be able to detect if we expect a next segment or not.
Comment 10 Florin Apostol 2015-07-08 19:41:29 UTC
Created attachment 307102 [details] [review]
replaced patch 5/5

corrected the offending test. It now works correctly with the latest master.
Comment 11 Sebastian Dröge (slomo) 2015-07-08 20:15:31 UTC
commit cd4755635111775ed2df1616e0410664b4869f81
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Jul 8 23:14:13 2015 +0300

    mpdparser: Fix some memory leaks in the MPD parser and unit test

commit d525d9c39145fed21a4a7c5dd6aae3a58f14372a
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Tue Jul 7 16:59:52 2015 +0100

    dashdemux: tests: added unit tests to test all functions
    
    Added unit tests for all functions. Code coverage:
    Overall coverage rate:
      lines......: 83.8% (1941 of 2316 lines)
      functions..: 100.0% (141 of 141 functions)
Comment 12 Florin Apostol 2015-07-08 21:01:49 UTC
what tools/tests did you run to spot the memory leaks? I'm using "make elements/dash_mpd.lcov" to build and run the unit tests and get the code coverage.
Comment 13 Sebastian Dröge (slomo) 2015-07-08 21:27:12 UTC
make elements/dash_mpd.valgrind