libdav1d itx_1d.c is not tested
Categories
(Core :: Audio/Video: Playback, defect, P3)
Tracking
()
People
(Reporter: mjf, Unassigned)
References
(Blocks 1 open bug)
Details
From a reviewbot comment in https://phabricator.services.mozilla.com/D67511 while updating dav1d to 0.6.0:
https://searchfox.org/mozilla-central/source/third_party/dav1d/src/itx_1d.c
|
Reporter | |
Updated•6 years ago
|
|
|
Reporter | |
Updated•6 years ago
|
Michael, just to make sure I'm understanding, this file has no coverage and we want some? Rather than that our tests are exercising this but coverage is not being detected correctly?
I've encountered some cases in the GMP where I'm fairly sure code is being run, but it's not detected by coverage tools, and am wondering if we have any similar issues with RDD.
|
|
Reporter | |
Comment 2•6 years ago
|
||
(In reply to Bryce Seager van Dyk (:bryce) from comment #1)
Michael, just to make sure I'm understanding, this file has no coverage and we want some? Rather than that our tests are exercising this but coverage is not being detected correctly?
I've encountered some cases in the GMP where I'm fairly sure code is being run, but it's not detected by coverage tools, and am wondering if we have any similar issues with RDD.
That is a good question. I assumed the tool was correct in knowing whether the code is being run or not. The exact quote from the tool is:
In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
third_party/dav1d/src/itx_1d.c
Should they have tests, or are they dead code ?
You can file a bug blocking Bug 1415824 for untested files that should be tested.
You can file a bug blocking Bug 1415819 for untested files that should be removed.
If you see a problem in this automated review, please report it here.
To me this sounded like the tool had already marked this code as "having no coverage" and since the 0.6.0 imported modified the file it was flagged. I don't know anything about our av1 test coverage so I can't speak to whether the tool has correctly determined whether tests are hitting this code. Let me know if I need to do more investigation.
|
||
Comment 3•6 years ago
|
||
Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)
|
||
Comment 4•6 years ago
|
||
Do we run libdav1d's tests? I haven't found any indication of that.
According to https://videolan.videolan.me/-/dav1d/-/jobs/416281/artifacts/coverage/index.src_itx_1d.c.html (I didn't find a stable link to the coverage report, this one expires after one week unfortunately), it is indeed coveraged (not all branches, but definitely much more than zero coverage).
I don't think we should bother to cover it by Mozilla-specific tests. Rather, the third-party/dav1d code should be excluded from that review bot analysis.
Description
•