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 774844 - dashdemux unit tests are disabled
dashdemux unit tests are disabled
Status: RESOLVED DUPLICATE of bug 781632
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-22 15:37 UTC by mariuszb
Modified: 2017-08-22 15:41 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description mariuszb 2016-11-22 15:37:58 UTC
Dashdemux' tests were disabled because new functionality changed its behavior. To me it looks like these changes were not fit for upstreaming until tests were accordingly adjusted and passing. Or I'm missing something?
Comment 1 Sebastian Dröge (slomo) 2016-11-22 15:39:46 UTC
The tests are fundamentally broken. They assume that dashdemux never ever looks at the actual data as they pass more or less random data through dashdemux.

dashdemux nowadays looks at the data and if the data is not valid, it will not behave correctly (i.e. the tests fails).


I think these "unit" tests should rather become gst-validate tests on proper streams.
Comment 2 mariuszb 2016-11-22 15:47:39 UTC
To me it looks like whoever changed the behavior made a breaking change and inherited the problem of making the tests capable of producing valid data. By disabling tests altogether how can one be sure what's the state of the code their pushing? 

So tests were not fundamentally broken because their were adequate to behavior of dashdemux. By the same token you could argue that current behavior is broken because without unit test how can you prove otherwise?
Comment 3 Sebastian Dröge (slomo) 2016-11-22 15:55:10 UTC
There are gst-validate tests for dashdemux already, but they don't cover everything that the unit test is testing yet.

It doesn't seem worthwhile to have the tests generate valid dash streams (for which we still miss a lot of features). It seems more useful to do these tests on real streams, which is exactly what gst-validate is for.
Comment 4 mariuszb 2016-11-22 16:04:19 UTC
Whether it's done on unit test or gst-validate level is irrelevant. What's relevant is that changes to dashdemux shouldn't land unless equivalent testing coverage was provided.
Comment 5 Tim-Philipp Müller 2016-12-13 17:11:28 UTC
I understand where you're coming from of course, but we simply don't have such a policy, i.e. to guarantee that there must never be any regression in testing coverage.

If we had such a policy we would have declared these tests not fit for purpose at the time and would never have merged these tests in the first place like that.

The tests were useful for a while, but unfortunately they were built on quicksand and now they don't work anymore for reasons that are internal to the way the tests were written.

It does not seem reasonable or desirable to me to block merging of new features and other improvements based on the condition that they fix up these deficient tests as well.

Testing coverage is an ongoing effort and on occasions like this where we unfortunately have to regress in test coverage we just have to hope that interested parties will sooner or later restore the missing coverage.

The most productive way forward would be to make a list of things that need to be tested, and try to figure out how and where to restore that coverage.

Most of the testing should probably move to the validate test suite working on real streams.

We should probably also add more streams to validate in general, like the official test vectors.
Comment 6 Tim-Philipp Müller 2017-08-22 15:41:20 UTC

*** This bug has been marked as a duplicate of bug 781632 ***