Closed Bug 1314514 Opened 6 years ago Closed 6 years ago
gtestify the cubeb unit tests
The cubeb unit tests use ScopedXPCOM (conditionally, if CUBEB_GECKO_BUILD is set) but they don't need it.
This patch removes it the ScopedXPCOM usage, which allows the tests to be changed from GeckoCppUnitTests to CppUnitTests. With this change, the only remaining use of CUBEB_GECKO_BUILD is in media/libcubeb/src/cubeb_resampler_internal.h.
Attachment #8806593 - Flags: review?(kinetik)
Comment on attachment 8806593 [details] [diff] [review] Remove unnecessary XPCOM dependency from cubeb unit tests Review of attachment 8806593 [details] [diff] [review]: ----------------------------------------------------------------- r+
Attachment #8806593 - Flags: review?(kinetik) → review+
This is harder than it first looked. The attached patch works on Linux, but fails to build on Mac, Windows and Android: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de81eb898308 Adding |USE_LIBS += ['xul']| fixed the problem on Mac but caused the tests to segfault on Linux. Converting these to gtests might be necessary.
> 70497380eb5351f967c3fa52f3eb432969c2e65d I backed this out for now. Will look at what's needed upstream to for GTest.
Pull request #189 upstream converts the tests to gtests and includes njn's removal of the XPCOM deps. https://github.com/kinetiknz/cubeb/pull/189
Updates in-tree libcubeb to e1da8042. This picks up the test conversions to gtests and subsequent minor tweaks to integrate correctly with Gecko, plus: 7ac08e0 Make API visible cubeb_device_info members and cubeb_devid const. 5abde56 Avoid using cubeb_devid internally. 37ce70d Bail out safely from the rendering loop if we could not join the rendering thread in time (#187) The last item is bug 1315194. Waiting on try results before requesting review: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca00ab2bb1a0d438eb3a2315574bb4cb6a4dcae
Attachment #8806593 - Attachment is obsolete: true
Fix OS X build with b46ffc5d from upstream.
Attachment #8809674 - Attachment is obsolete: true
Above fix, plus more tests enabled (including gtest, whoops!): https://treeherder.mozilla.org/#/jobs?repo=try&revision=52fc9aab55249af421309baba93a35c6a2e543a7
Thank you for doing this. One small nit: gtests are usually put in a directory called "gtest". I suggest renaming media/libcubeb/tests as media/libcubeb/gtest.
Rename media/libcubeb/tests to gtest.
Attachment #8809692 - Attachment is obsolete: true
cubeb.duplex and cubeb.record are failing because they expect to capture sound from a device. I guess these were previously running in a test context where a dummy capture device was available? Paul, do you know anything about this?
(Paul, see comment 12)
(In reply to Matthew Gregan [:kinetik] from comment #12) > cubeb.duplex and cubeb.record are failing because they expect to capture > sound from a device. > > I guess these were previously running in a test context where a dummy > capture device was available? Paul, do you know anything about this? I learned very recently that some time ago, changes were made to the way one should indicate that a `cppunittest` should be ran during the `cpp` chunk. It is now necessary to list it in . Since we haven't been informed, and because the list has been made in October 2014, we're not running the unit-tests that use input (cubeb was not capable of doing input at the time). When I discovered this (randomly by trying to enable more cubeb tests on try and not seeing them in the log), I've been pushing to try with more tests enabled . However because of a wrong base revision and a bug in trychooser (that prevents executing cppunittests on their own), it errored and went back in my queue. Sad state of affair. However there is some light, I've been researching ways to test cubeb better and I found ways to do a lots of things on OSX and Windows. I'll open issues upstream. : http://searchfox.org/mozilla-central/source/testing/cppunittest.ini : https://treeherder.mozilla.org/#/jobs?repo=try&revision=0dfdfd5da55fa4bb626194c4bf872f0a01594de9
Thanks! I wasn't aware of cppunittest.ini. I've removed the old tests from that now. So the easy solution for now is to disable those tests in the gtests as well. Also updated the libcubeb import to latest master to pick up the PulseAudio crash fix and AudioUnit race fix. And renamed tests to gtest (and remembered to update the commit) this time. This should be ready to land, assuming Try looks good.
And with the actual patch rather than the empty try commit.
Attachment #8810673 - Flags: review?(padenot) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/359999c77a46 Update libcubeb to 8bab182c. r=padenot
This caused build failures like https://treeherder.mozilla.org/logviewer.html#?job_id=39237720&repo=mozilla-inbound Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ea07c491499006c6650fbd75431a393c4f4d124d
Ah, ALLOWED_XPCOM_GLUE was added after the base cset I was pushing to try.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e66c6309387c Update libcubeb to 8bab182c. r=padenot
Summary: Remove unnecessary XPCOM dependency from cubeb unit tests → gtestify the cubeb unit tests
You need to log in before you can comment on or make changes to this bug.