Bug 160736 – [GTK] run-gtk-tests should use webkitpy.port.gtk and webkitpy.port.xvfbdriver

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

The script run-gtk-tests implements its own _run_xvfb() and its own _setup_testing_environment() instead of relying on webkitpy.port.xvfbdriver:XvfbDriver and webkitpy.port.gtk:GtkPort::setup_environ_for_server()


As a result of this, the glx/egl software rasterizer built by the jhbuild is not used, defaulting to use the system one.


This is causing errors on the ARM bot, that seems don't has a proper one shipped by the system:

https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/11833/steps/API%20tests/logs/stdio


ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/WebViewTest.cpp:58:static gboolean WebViewTest::webProcessCrashed(WebKitWebView*, WebViewTest*): code should not be reached

FAIL

GTester: last random seed: R02Sd767ea08b233d8032c8a8ba1cb7a7bf8

(pid=3374)

  /webkit2/WebKitWebView/menu-dismissed:                               libEGL warning: GLX/DRI2 is not supported

libEGL warning: DRI2: failed to open swrast (search paths /usr/lib/arm-linux-gnueabihf/dri:${ORIGIN}/dri:/usr/lib/dri)

Xlib: sequence lost (0x10096 > 0x97) in reply type 0x0!

[Carlos Alberto Lopez Perez]

Created attachment 286713 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 286713 [details]
Patch

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

Excellent! I have only a couple of comments

> Tools/Scripts/run-gtk-tests:186
> +        port_name = "gtk"
> +        self._port = Host().port_factory.get(port_name, port_options)

port_name is only used here, you could probably pass gtk directly to the factory method. Also, this is called create_driver but it's also creating the port, I would move this to the __init__ instead, and create_driver should assume self_port already exists.

self._port = Host().port_factory.get('gtk')
self._driver = self._create_driver()

no need to create and empty port_options list if it's not really used.

> Tools/Scripts/run-gtk-tests:190
> +            self._port._nativexorg = True
> +        if self._options.use_wayland:
> +            self._port._wayland = True

When wayland is true, whether using xvfb or not is just ignored, so I would make it clearer that here with something like:

if self._options.use_wayland:
    self._port._wayland = True
else:
    self._port._nativexorg = not self._options.use_xvfb

> Tools/Scripts/run-gtk-tests:193
> +        if not driver.check_driver(self._port):
> +            raise RuntimeError("Failed to check driver %s" %driver.__class__.__name__)

This is not handled anywhere. In this case we should ensure the script finishes with an exit error code. At the moment the constructor is not expected to fail and we do:

sys.exit(TestRunner(options, args).run_tests())

now we should create the TesRunner first inside a try except block and exit early if the creation fails, and then sys.exit(runner.run_tests())

[Carlos Alberto Lopez Perez]

Created attachment 286865 [details]
Patch

Patch for landing addressing reviewer comments. Will not land until blocking bug 161149 is also ready for land. Otherwise the bots could hand without the fix from bug 161149

[Carlos Alberto Lopez Perez]

Committed r205014: <http://trac.webkit.org/changeset/205014>