GNOME Bugzilla – Bug 774844
dashdemux unit tests are disabled
Last modified: 2017-08-22 15:41:20 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?
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.
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?
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.
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.
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.
*** This bug has been marked as a duplicate of bug 781632 ***