Bug 101261 – test-webkitpy shows failure output on my linux box

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

% test-webkitpy
Suppressing most webkitpy logging while running unit tests.
Skipping tests in the following modules or packages because they are really, really, slow:
    webkitpy.common.checkout.scm.scm_unittest
    (https://bugs.webkit.org/show_bug.cgi?id=31818; use --all to include)

[100/1598] webkit2.messages_unittest.HeaderTest.test_header (+15)Failure: No such entity  
Failure: No such entity
Failure: No such entity
Ran 1598 tests in 3.133s                                                                                                         

OK

Unclear if it's webkit2-code related or not yet.

[Dirk Pranke]

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?

[Peter Beverloo]

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

[Peter Beverloo]

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.

[Peter Beverloo]

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.

[Dirk Pranke]

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.

[Dominik Röttsches (drott)]

(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?

[Zan Dobersek]

(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?

[Dirk Pranke]

(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.

[Zan Dobersek]

Created attachment 181697 [details]
Patch

[Peter Beverloo]

(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.

[Eric Seidel (no email)]

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?

[Zan Dobersek]

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.

[Eric Seidel (no email)]

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.

[Zan Dobersek]

(In reply to comment #13)
> But that's a separate bug.

Filed bug #106354.

[Zan Dobersek]

Comment on attachment 181697 [details]
Patch

Clearing flags on attachment: 181697

Committed r139084: <http://trac.webkit.org/changeset/139084>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.