Bug 106354 – [EFL][GTK] Make the PulseAudioSanitizer an object on the EflPort, GtkPort

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zan Dobersek]

As Eric put it in https://bugs.webkit.org/show_bug.cgi?id=101261#c13
> 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. :)

The EflPort and GtkPort interfaces currently inherit from PulseAudioSanitizer. This also makes the mocking of the latter in unit tests a bit clearer.

[Zan Dobersek]

Created attachment 181721 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 181721 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181721&action=review

Thanks.

> Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
>      def setup_test_run(self):
> -        self._unload_pulseaudio_module()

Should this be calling super?

[Zan Dobersek]

(In reply to comment #2)
> > Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
> >      def setup_test_run(self):
> > -        self._unload_pulseaudio_module()
> 
> Should this be calling super?

That method in layout_tests.port.base.Port currently does nothing, but it's still probably a good idea to call it.
https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L853

A new patch or can I just put the call in in this one? What works best for you?

[Eric Seidel (no email)]

(In reply to comment #3)
> (In reply to comment #2)
> > > Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
> > >      def setup_test_run(self):
> > > -        self._unload_pulseaudio_module()
> > 
> > Should this be calling super?
> 
> That method in layout_tests.port.base.Port currently does nothing, but it's still probably a good idea to call it.
> https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L853
> 
> A new patch or can I just put the call in in this one? What works best for you?

Doesn't matter to me in the slightest. :)  I'm happy to approve it either way.

[Zan Dobersek]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > > Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
> > > >      def setup_test_run(self):
> > > > -        self._unload_pulseaudio_module()
> > > 
> > > Should this be calling super?
> > 
> > That method in layout_tests.port.base.Port currently does nothing, but it's still probably a good idea to call it.
> > https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L853
> > 
> > A new patch or can I just put the call in in this one? What works best for you?
> 
> Doesn't matter to me in the slightest. :)  I'm happy to approve it either way.

The ChromiumPort interface behaves the same way, so a new patch is more appropriate.

[Zan Dobersek]

Comment on attachment 181721 [details]
Patch

Clearing flags on attachment: 181721

Committed r139095: <http://trac.webkit.org/changeset/139095>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.