1257713 - Convert signaling unit tests to gecko gtests

Status: RESOLVED DUPLICATE of bug 1316611

(Core :: WebRTC: Signaling, defect, P1)

Description

[Eric Rahm [:erahm]]

Similar to what we did in bug 1239870 we should convert the signaling unit tests to gecko gtests. This will allow us to remove various "fake" implementations as well as the few remaining references to |PR_LogModuleInfo|.

We can probably just reuse the MtransportTest [1] base class to handle various bits of initialization.

I think it will also allow us to get rid of the |mtransport_s| lib [2].

[1] https://dxr.mozilla.org/mozilla-central/rev/3e04659fdf6aef792f7cf9840189c6c38d08d1e8/media/mtransport/test/gtest_utils.h#103
[2] https://dxr.mozilla.org/mozilla-central/rev/3e04659fdf6aef792f7cf9840189c6c38d08d1e8/media/mtransport/testlib/moz.build

Comment 1

[Eric Rahm [:erahm]]

Okay, so it looks like mediaconduit_unittests is currently busted, I'll convert it, but I can't verify I didn't break anything:

> [erahm@mozilla21268 mozilla-central]$ MOZ_WEBRTC_MEDIACONDUIT_TESTS=1 ./mach cppunitests obj-x86_64-unknown-linux-gnu-clang/dist/bin/mediaconduit_unittests 
> We're assuming the 'cppunitests' command is 'cppunittest' and we're executing it for you.
> 
> SUITE-START | Running 1 tests
> TEST-START | mediaconduit_unittests
> PROCESS | 30126 | 
> Assertion failure: ((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1))), at /home/erahm/dev/mozilla-central/media/mtransport/test/mtransport_test_utils.h:31
> TEST-UNEXPECTED-FAIL | mediaconduit_unittests | test failed with return code -139
> TEST-INFO took 117ms
> SUITE-END | took 0s
> Result summary:
> cppunittests INFO | Passed: 0
> cppunittests INFO | Failed: 1

Comment 2

[Eric Rahm [:erahm]]

mediapipeline_unittest uses "FakeMediaStreams", I can't just swap that out as this seems to have drifted from the current DOM API. Leaving it as-is (using a fake impl) doesn't work either as there are some symbol conflicts for redefining MSG etc.

I can update it to be a gecko gtest, but it won't actually build.

Comment 3

[Eric Rahm [:erahm]]

Attachment: Convert signaling tests to gecko gtests

Comment 4

[Eric Rahm [:erahm]]

Attachment: WIP Pt 1: Convert mediapipline unit test

This is the initial work to get the mediapipline test converted. Some work will
have to be done to figure out what to do with the FakeMediaStream interfaces
that have diverged from the non-fake interfaces.

Comment 5

[Eric Rahm [:erahm]]

Attachment: WIP Pt 2: Convert the signaling unit test

This is the initial work to get the siginaling test converted. Some work will
have to be done to figure out what to do with the FakeMediaStream interfaces
that have diverged from the non-fake interfaces.

Comment 6

[Eric Rahm [:erahm]]

Alright I've taken this about as far as I can. The mediapipline and signaling unit tests remain partially converted. They both use some sort of pseudo-fake-mock impls that have diverged from their DOM counterparts. Someone with more media knowledge would need to convert those.

Byron, it looks like you win the touched those files the most award, any chance you could take on the final part or give rather concrete pointers on how to fix the fake impl situation?

Comment 7

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b78e84f5192

Comment 8

[Byron Campen [:bwc]]

Looking into it.

Comment 9

[Byron Campen [:bwc]]

So, I know as much as you do about the fake media stream stuff. It might be better to ping someone over on the media side of things to give some pointers. They're more likely to know stuff like how to get a media source without using the mock stuff.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

If these will be linked in with main XUL/etc, then we can just drop the fake impls and use real mediastreams.  We used the fake impls because of (IIRC) linkage issues.  

Using real impls likely will require some fixes to the tests, which may be what erahm ran into.  We should also be able to turn on more-real video in some of the tests.

Comment 11

[Dan Minor [:dminor]]

Forward duping this to Bug 1316611. I'm planning to tackle the tests one or two at a time as dependencies of that bug rather than trying to fix everything in one go. The work in progress here will be helpful for that.