Summary: | test-webkitpy shows failure output on my linux box | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||
Component: | Tools / Tests | Assignee: | Zan Dobersek <zan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, dpranke, d-r, peter, rniwa, sam, webkit.review.bot, zan | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Eric Seidel (no email)
2012-11-05 14:46:24 PST
It runs cleanly for me on my linux box synced to r133537. There were some wk2-related failures briefly last week; is it possible you're synced to one of those revisions? I'm seeing this as well. A Google search on the error shows that it's likely PulseAudio causing this. Are you on Precise? The test it shows the failure messages show on differs between tests. I'm not seeing it when executing "test-webkitpy -j1" either, and with -j2 it shows once. ---------- [162/1614] webkitpy.common.net.buildbot.buildbot_unittest.BuilderTest.test_failure_and_flaky passedFailure: No such entity Failure: No such entity Failure: No such entity Ran 1614 tests in 3.034s OK I'm seeing this consistently when running webkitpy.layout_tests.port.efl_unittest and gtk_unittest. Both ports use the PulseAudioSanitizer class to unload the "module-stream-restore" PulseAudio module before the test run (as part of setup_test_run()), and restore it again as part of clean_up_test_run(). This executes the following commands: $ pactl list short modules $ pactl unload-module [index of module-stream-restore module] .. test run .. $ pactl load-module module-stream-resource Adding a print right before the call to unload-module shows that this is being executed twice (or more), so we're running into a concurrency issue here. The class was added in http://wkrev.com/115478. To clarify: two or more threads are simultaneously running PulseAudioSanitizer._unload_pulseaudio_module(), all execute $ pactl list short modules ..and all see that the "module-stream-resource" module is loaded, causing them try to unload it. The first unload succeeds, subsequent unloads show the "Failure: No such entity" message. Easy solution would be to protect the call to _unload_pulseaudio_module and _restore_pulseaudio_module in a multiprocess.Lock(), but this would still mean that we're loading and unloading the module two times per webkitpy test run. Dominik, do you have any preferred solution? IIRC running failed tests again is seen as a separate run, so in case of failed layout tests the PulseAudio module would also be loaded and unloaded twice. Note that you can mark python unit tests in webkitpy as needing to be run in isolation by prepending "serial_" to the test name. I would do that rather than do locking manually. (In reply to comment #4) > Dominik, do you have any preferred solution? IIRC running failed tests again is seen as a separate run, so in case of failed layout tests the PulseAudio module would also be loaded and unloaded twice. Do you think what Dirk suggests would be the right fix, or do you mean that you see this when running the unit tests but you consider it an issue in other situations, too? (In reply to comment #4) > Dominik, do you have any preferred solution? IIRC running failed tests again is seen as a separate run, so in case of failed layout tests the PulseAudio module would also be loaded and unloaded twice. But that isn't done concurrently, so there's no 'No such entity' output in that case (AFAIK). I'm not sure there's a way of enforcing running unit tests from two separate files separately. How about mocking out the PulseAudioSanitizer methods? (In reply to comment #7) > I'm not sure there's a way of enforcing running unit tests from two separate files separately. How about mocking out the PulseAudioSanitizer methods? The serial_ feature works across files. Of course, mocking out the routines would probably work as well. Created attachment 181697 [details]
Patch
(In reply to comment #5) > Note that you can mark python unit tests in webkitpy as needing to be run in isolation by prepending "serial_" to the test name. I would do that rather than do locking manually. In this case it's a few unit tests for both the EFL and GTK ports that call setup_test_run(), I'm not sure if running them serially is the right solution as it would still unload and restore the module. (In reply to comment #7) > (In reply to comment #4) > > Dominik, do you have any preferred solution? IIRC running failed tests again is seen as a separate run, so in case of failed layout tests the PulseAudio module would also be loaded and unloaded twice. > > But that isn't done concurrently, so there's no 'No such entity' output in that case (AFAIK). > > I'm not sure there's a way of enforcing running unit tests from two separate files separately. How about mocking out the PulseAudioSanitizer methods? The module is being unloaded concurrently as two or more tests are invoking setup_test_run(), and thus the PulseAudio unload method, at the same time. I think Zan's solution makes sense, and it stops the unit tests from unloading and restoring the PulseAudio module. Thank you for looking in to it -- it solves the issue for me. Comment on attachment 181697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181697&action=review Seems fine. Makes me think that this Port object may be doing too much. :) > Tools/Scripts/webkitpy/layout_tests/port/efl_unittest.py:43 > + port._unload_pulseaudio_module = lambda: None > + port._restore_pulseaudio_module = lambda: None I guess there is no PulseAudio object on these ports that you can just mock? The port itself handles teh loading/unloading (and presumably actions with) this code? Comment on attachment 181697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181697&action=review >> Tools/Scripts/webkitpy/layout_tests/port/efl_unittest.py:43 >> + port._restore_pulseaudio_module = lambda: None > > I guess there is no PulseAudio object on these ports that you can just mock? The port itself handles teh loading/unloading (and presumably actions with) this code? The port interfaces inherit from the PulseAudioSanitizer and call these methods on test run setup/cleanup. https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/efl.py#L37 https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py#L38 Please give the final thumbs-up for landing this if all seems fine to you. This is fine. But eventually we should fix that. A port "is not" a PulseAudioSanitizer, it might "have a", but the usage of inheritance here is wrong IMO. :) But that's a separate bug. (In reply to comment #13) > But that's a separate bug. Filed bug #106354. Comment on attachment 181697 [details] Patch Clearing flags on attachment: 181697 Committed r139084: <http://trac.webkit.org/changeset/139084> All reviewed patches have been landed. Closing bug. |