Bug 161149 - [GTK] run-gtk-tests should use the driver environment for checking the accessibility bus
Summary: [GTK] run-gtk-tests should use the driver environment for checking the access...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords:
Depends on:
Blocks: 160736
  Show dependency treegraph
 
Reported: 2016-08-24 10:40 PDT by Carlos Alberto Lopez Perez
Modified: 2016-08-26 09:38 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.16 KB, patch)
2016-08-24 10:56 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (4.17 KB, patch)
2016-08-24 11:10 PDT, Carlos Alberto Lopez Perez
cgarcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2016-08-24 10:40:23 PDT
Currently run-gtk-tests waits for the accessibility bus on D-Bus to appear like this:

        # We need to wait until the SPI bus is launched before trying to start the SPI
        # registry, so we spin a main loop until the bus name appears on DBus.
        loop = GLib.MainLoop()
        Gio.bus_watch_name(Gio.BusType.SESSION, 'org.a11y.Bus', Gio.BusNameWatcherFlags.NONE,
                           lambda *args: loop.quit(), None)
        loop.run()


The problems are:

 1) It runs the check using the environment from the user rather than the environ that should be used for the tests. So the environment variables from the driver (after bug 160736) are not correctly set (like DISPLAY or WAYLAND_DISPLAY). This seems to be somehow relevant, because if the system don't has the environment variable DBUS_SESSION_BUS_ADDRESS defined, dbus is able to do the right thing if at least DISPLAY is set to the correct value. Otherwise it fails.

 2) It don't has a timeout. In case the accessibility bus don't appears, it hangs forever.
Comment 1 Carlos Alberto Lopez Perez 2016-08-24 10:56:52 PDT
Created attachment 286869 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2016-08-24 11:10:12 PDT
Created attachment 286871 [details]
Patch

trivial: fix identation
Comment 3 Carlos Garcia Campos 2016-08-24 23:41:20 PDT
Comment on attachment 286871 [details]
Patch

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

> Tools/Scripts/run-gtk-tests:155
> +    def _wait_for_accessibility_bus(self, timeout=5, env={}):

Why are params optional if you are passing them always? If the timeout can't be configured, we don't need to pass it to the function, use 5 directly in GLib.timeout_add_seconds

> Tools/Scripts/run-gtk-tests:191
> +        if not self._wait_for_accessibility_bus(timeout=5, env=self._test_env):

You don't need to use the name=value, just pass the values. But in this case, you are passing a hardcoded value that could be used directly by the function, and a member variable to a member function, so the function could simply use self._test_env
Comment 4 Carlos Alberto Lopez Perez 2016-08-26 09:38:20 PDT
Committed r205017: <http://trac.webkit.org/changeset/205017>