[Webkit-unassigned] [Bug 71662] [GTK] media/event-attributes.html fails

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=71662





--- Comment #15 from Simon Pena <spena at igalia.com>  2012-04-11 01:34:10 PST ---
(From update of attachment 136439)
View in context: https://bugs.webkit.org/attachment.cgi?id=136439&action=review

>> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:60
>> +        if self._worker_number == 0:
> 
> Why are you doing this change here at all, and why checking if _worker_number == 0? 
> 
> If you want this to happen before *any* tests are run (and reset after *all* of them have run), you should do this work in GtkPort.setup_test_run (and it looks like there isn't a corresponding clean_up_test_run(), so we should probably add that and add the call in Manager._clean_up_run in manager.py.
> 
> On the other hand, if you want to do this for each individual test, it's not clear to me how this would work when running multiple DRTs simultaneously?

Thanks! I wanted that to happen before tests were run, so setup_test_run and clean_up_test_run seem the way to go. I'll update the patch accordingly.

I'll Also follow Martin and Philippe comments regarding to code.

>>> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:65
>>> +            for module in modules_list.split('\n'):
>> 
>> Please use splitlines()
> 
> It makese sense to split this off into a helper method.

I'll fix this in the new version of this patch.

>> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:70
>> +                subprocess.Popen(["pactl", "unload-module", self._module_index], stdout=None, stderr=None)
> 
> stdout and stderr are None by default, I think. So no need to set them here.

OK, I'll get that fixed in the next version.

>> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:81
>> +                subprocess.Popen(["pactl", "load-module", "module-stream-restore"], stdout=devnull, stderr=devnull)
> 
> Is there a way to disable this for a single process only, so we can avoid putting the system in a bad state if the process crashes?

Dirk comment explained me how to do it, so I'll get it fixed in the next version of the patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.