Verify coverage for WebRTC code (in e10s mode) is generated

RESOLVED FIXED

Status

Testing
Code Coverage
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: ekyle, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 months ago
From nohlmeier,

> In case of WebRTC e10s is only covered through mochitest, not through 
> any unit tests. But codecov.io shows all the networking code for e10s 
> as completely uncovered. That makes me a little bit suspicious of 
> whats going on there.

Comment 1

7 months ago
Thanks Kyle.

What caught my attention is low coverage here: https://codecov.io/gh/marco-c/gecko-dev/src/master/media/mtransport/nr_socket_prsock.cpp

nr_socket_prsock.cpp contains the some level network access code for WebRTC. The one file contains code non-e10s, which is basically the NrSocket class which appears as covered. And then their are the NrUdpSocketIpc and NrTcpSocketIpc classes which appear to have no coverage. These two classes having no coverage is highly implausible.

The big difference between NrSocket and the two other classes is that NrSocket gets covered by lots of low level gtest test in /media. But the two other classes are only covered by e10s mochitest in mda*.
The second important piece here might be that the two e10s classes are executed in the content process and not in the parent process. I don't know if that makes a difference for CC.

Let me know if I can help in any way with CC.

Comment 2

7 months ago
One more specific information: the WebRTC mochitest are located at /dom/media/tests/mochitest/*.
As indicated before they should get executed in mda2 I believe, for sure in one of the mda* chunks.
e10s tests are not running yet in the code coverage build, so this is expected.

I'll leave this open, so we can verify it once bug 1314305 is fixed.
No longer blocks: 1314305
Depends on: 1314305
Nils, the class that you mentioned appears to be covered in the report I manually generated with the patch from bug 1314305: https://github.com/marco-c/code-coverage-reports/blob/master/mochitest-media-e10s.tar.bz2.

Can you confirm that a few other WebRTC-related things you expect to be covered are actually covered?
Flags: needinfo?(drno)

Comment 5

7 months ago
Yes a brief inspection looks good. I see in that report the e10s pieces covered. The code which is used with and without e10s shows coverage. And the non-e10s only code shows no coverage.
Now your big task is: how merge these numbers into one report :-)
Flags: needinfo?(drno)
Thanks! I'll mark this FIXED when bug 1314305 is fixed.
We enabled e10s builds in a bug blocking bug 1314305. The report looks fine to me now: https://codecov.io/gh/marco-c/gecko-dev/src/544308c1475fe4b2fbc3878cea5a4c7df21a5cef/media/mtransport/nr_socket_prsock.cpp#L1137.
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED

Comment 8

7 months ago
(In reply to Marco Castelluccio [:marco] from comment #7)
> We enabled e10s builds in a bug blocking bug 1314305. The report looks fine
> to me now:
> https://codecov.io/gh/marco-c/gecko-dev/src/
> 544308c1475fe4b2fbc3878cea5a4c7df21a5cef/media/mtransport/nr_socket_prsock.
> cpp#L1137.

Yes this look good. And this coverage can only be achieved by running non-e10s + e10s tests!
Cool progress!

Looking forward to chase the WebRTC test gaps :-)
You need to log in before you can comment on or make changes to this bug.