It's becoming harder and harder to run GTK+ API tests outside of run-gtk-tests. 'make dist' should be able to use run-gtk-tests so that we can rely on some environment variables and Xvfb to be around.
Created attachment 132395 [details] Patch
This bug is on track for WebKitGTK+ 1.8.
Comment on attachment 132395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132395&action=review Nice patch! Just a few remarks to consider before landing :) > Source/WebKit/gtk/GNUmakefile.am:-639 > - $(GTESTER) --verbose $$test_options -o test-report.xml $(TEST_PROGS); \ > - $(GTESTER_REPORT) test-report.xml > test-report.html ; Would it be useful to add support for the test-report generation in run-gtk-tests? > Tools/Scripts/run-gtk-tests:122 > + if (not spi_bus_launcher_path) or (not spi_registryd_path): You can just omit the parenthesis here I think. > Tools/Scripts/run-gtk-tests:126 > + uself._ally_bus_launcher = self._create_process([spi_bus_launcher_path], env=self._test_env) typo: uself > Tools/Scripts/run-gtk-tests:139 > + if spi_registryd_path: I don't think this test is needed, it's done in line 122 already. > Tools/Scripts/run-gtk-tests:182 > + tests_to_remove = [] > + for test in self._tests: > + for skipped in self._skipped_tests: > + if test.find(skipped) != -1: > + tests_to_remove.append(test) > + continue Maybe you can simplify this code by using sets, if you can avoid using .find() to filter, something like: tests = sets.Set(self._tests) self._tests = tests.difference(self._skipped_tests)
Comment on attachment 132395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132395&action=review >> Source/WebKit/gtk/GNUmakefile.am:-639 >> - $(GTESTER_REPORT) test-report.xml > test-report.html ; > > Would it be useful to add support for the test-report generation in run-gtk-tests? It might be, though I'm not aware of anyone ever using this feature. It could be a feature of run-gtk-tests instead if people are interested. >> Tools/Scripts/run-gtk-tests:122 >> + if (not spi_bus_launcher_path) or (not spi_registryd_path): > > You can just omit the parenthesis here I think. Okay. >> Tools/Scripts/run-gtk-tests:126 >> + uself._ally_bus_launcher = self._create_process([spi_bus_launcher_path], env=self._test_env) > > typo: uself Yikes! Thanks. >> Tools/Scripts/run-gtk-tests:139 >> + if spi_registryd_path: > > I don't think this test is needed, it's done in line 122 already. Fixed. >> Tools/Scripts/run-gtk-tests:182 >> + continue > > Maybe you can simplify this code by using sets, if you can avoid using .find() to filter, something like: > > tests = sets.Set(self._tests) > self._tests = tests.difference(self._skipped_tests) The problem with sets is that I want to do a substring search. Is it possible easily? I considered using lisp-y list manipulation like map, but this seemed clearer in the end. I have to land this patch now, but if you can think of a better way that involves substrings, I'll gladly update this code in a followup patch. Thanks for the review!
Committed r111316: <http://trac.webkit.org/changeset/111316>