Between at-spi-bus-launcher being slow at setting up the a11y bus and testatk trying to poke it too fast I think. One way to fix this is to make run-gtk-tests wait the bus have been started before launching the tests.
Also, when testatk fails to connect, it blocks the whole thing for 20 minutes...
Nice catch. We definitely need to fix this, for the sake of the bots!
I started a patch, making the script run a glib main loop and connecting to the bus's NameOwnerChanged to wait on the a11y bus. I'll rewrite it using gdbus instead of python-dbus.
Created attachment 130558 [details] Patch
Created attachment 130569 [details] Patch
Comment on attachment 130569 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130569&action=review Nice work, though I think this could use a little cleanup. > Tools/Scripts/run-gtk-tests:22 > +import os, sys, time Each import should have its own line. > Tools/Scripts/run-gtk-tests:24 > + > +from gi.repository import Gio, GLib No newline necessary here. > Tools/Scripts/run-gtk-tests:88 > + def _wait_dbus_service_and_run(self, service_name, handler): _run_command_when_dbus_service_appears? How are timeouts handled here? > Tools/Scripts/run-gtk-tests:113 > + global timed_out; Instead of a global variable, why not an instance variable? > Tools/Scripts/run-gtk-tests:131 > + def check_bailout(): You should probably rename this to something like: "check_if_tests_have_timed_out" > Tools/Scripts/run-gtk-tests:138 > + r = False > + if now - start > TIMEOUT: > + sys.stdout.write("Tests timed out after %d seconds\n" % TIMEOUT) > + sys.stdout.flush() > + r = True > + return r You can just use an early return here and avoid the variable: if now.time() - start <= TIMEOUT: return True sys.stdout.write("Tests timed out after %d seconds\n" % TIMEOUT) sys.stdout.flush() return False > Tools/Scripts/run-gtk-tests:152 > + # Make sure the accessibility registry daemon is running. > + a11y_registryd_running = False > + a11y_registryd_path = self._lookup_atspi2_binary(jhbuild_path, 'at-spi2-registryd') > + if a11y_registryd_path: > + try: > + a11y_registryd = Executive().popen([a11y_registryd_path], env=test_env) > + a11y_registryd_running = True > + except: > + sys.stderr.write("Failed to run the accessibility registry\n") > + sys.stderr.flush() > + This could probably be a helper. You could avoid the comment that way: def _ensure_accessibility_daemon_is_running(self): > Tools/Scripts/run-gtk-tests:161 > + if check_bailout(): > + timed_out = True > + break This doesn't seem to work in situations where the test never halts, right?
(In reply to comment #6) > > > Tools/Scripts/run-gtk-tests:88 > > + def _wait_dbus_service_and_run(self, service_name, handler): > > _run_command_when_dbus_service_appears? > > How are timeouts handled here? > Hum I don't see what could time out here. Can you elaborate please? > > > Tools/Scripts/run-gtk-tests:161 > > + if check_bailout(): > > + timed_out = True > > + break > > This doesn't seem to work in situations where the test never halts, right? True :( I wish gtester had some way to handle tests timing out.
Created attachment 130627 [details] Patch
Comment on attachment 130627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130627&action=review Looks good, but consider converting the a11y stuff to use with. Perhaps this could be a follwoup patch. > Tools/Scripts/run-gtk-tests:149 > + def run_for_real(): Maybe run_for_real could be called run_tests. :) > Tools/Scripts/run-gtk-tests:164 > + if self._a11y_registryd: > + self._a11y_registryd.terminate() > + > + a11y_bus_launcher.terminate() It seems we want to be really sure that the a11y stuff is cleaned up properly when exiting the loop, so I think it makes sense to use "with" here. http://docs.python.org/release/2.5/whatsnew/pep-343.html Without using this, we run the risk of an exception preventing the termination of the a11y stuff. I think I've seen this happen to me.
Committed r110059: <http://trac.webkit.org/changeset/110059>
(In reply to comment #9) > (From update of attachment 130627 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=130627&action=review > > Looks good, but consider converting the a11y stuff to use with. Perhaps this could be a follwoup patch. > I'll try this in a follow-up patch, thanks!