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

Status: RESOLVED FIXED

(Testing :: Code Coverage, enhancement)

Description

[Kyle Lahnakoski [:ekyle]]

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

[Nils Ohlmeier [:drno]]

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

[Nils Ohlmeier [:drno]]

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.

Comment 3

[Marco Castelluccio [:marco]]

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.

Comment 4

[Marco Castelluccio [:marco]]

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?

Comment 5

[Nils Ohlmeier [:drno]]

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 :-)

Comment 6

[Marco Castelluccio [:marco]]

Thanks! I'll mark this FIXED when bug 1314305 is fixed.

Comment 7

[Marco Castelluccio [:marco]]

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.

Comment 8

[Nils Ohlmeier [:drno]]

(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 :-)