Bug 64086

Created attachment 99977 [details] proposed fix

Comment on attachment 99977 [details] proposed fix OK. There is a similar "is_redhat_based()" function in the Gtk port. Please add a test for this in qt_unittest.py. Should be very easy to do with MockFileSystem({'/etc/debian_version', ''}) Something like this: def test_is_debian_based(self) port = self.make_port() # This is available in PortTestCase which QtPortTest isn't yet, I don't think, but could be made to self.assertEqual(port.is_debian_based(), False) port._filesystem = MockFileSystem({'/etc/debian_version', ''}) self.assertEqual(port.is_debian_based(), True) One could also test that overriding _is_debian_based() with lamba: False or lambda: True affected port._path_to_appache_config_file().basename() if you wanted. In any case, we just need some sort of testing.

Comment on attachment 99977 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=99977&action=review > Tools/Scripts/webkitpy/layout_tests/port/qt.py:32 > +import os Don't need this once you use FileSystem. > Tools/Scripts/webkitpy/layout_tests/port/qt.py:69 > + if os.path.isfile('/etc/debian_version'): Oh, this is wrong too. This should use self._filesystem.exists() or isfile if that method exists on FileSystem. We don't ever talk to os directly if we can help it. (Makes mockng hard.)

Same or similar bug on Qt Mac bot: 2011-07-11 04:15:11,000 86746 apache_http_server.py:182 DEBUG Starting httpd server, cmd="/usr/sbin/httpd -f "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/layout-test-results/httpd.conf" -C 'DocumentRoot "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/LayoutTests/http/tests"' -c 'Alias /js-test-resources "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/LayoutTests/fast/js/resources"' -c 'Alias /media-resources "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/LayoutTests/media"' -C 'Listen 127.0.0.1:8000' -C 'Listen 127.0.0.1:8081' -c 'TypesConfig "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/LayoutTests/http/conf/mime.types"' -c 'CustomLog "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/layout-test-results/access_log.txt" common' -c 'ErrorLog "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/layout-test-results/error_log.txt"' -C 'User "buildbot"' -c 'PidFile /tmp/WebKit/httpd.pid' -k start -c 'SSLCertificateFile /buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/LayoutTests/http/conf/webkit-httpd.pem'" Traceback (most recent call last): File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 438, in <module> Ignoring unsupported option: --use-remote-links-to-tests sys.exit(main()) File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 433, in main return run(port_obj, options, args) File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 112, in run num_unexpected_results = manager.run(result_summary) File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 874, in run interrupted, keyboard_interrupted, thread_timings, test_timings, individual_test_timings = self._run_tests(self._test_files_list, result_summary) File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 723, in _run_tests self.start_servers_with_lock() File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 936, in start_servers_with_lock self._port.start_http_server() File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/port/base.py", line 669, in start_http_server server.start() File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base.py", line 86, in start self._pid = self._spawn_process() File "/buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server.py", line 185, in _spawn_process raise http_server_base.ServerError('Failed to start %s: %s' % (self._name, err)) webkitpy.layout_tests.servers.http_server_base.ServerError: Failed to start httpd: httpd: Syntax error on line 197 of /buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/layout-test-results/httpd.conf: Cannot load /usr/lib/apache2/modules/mod_mime.so into server: dlopen(/usr/lib/apache2/modules/mod_mime.so, 10): image not found

Assigning this bug to Kristóf, since he has more knowledge of python than I have.

Comment on attachment 99977 [details] proposed fix Marking my patch obsolete.

I tried to disable NRWT on qt-mac platform: http://trac.webkit.org/changeset/90746 isDarwin() checks $^O, and it should be "darwin" on the Mac bot. Dihan, could you check what is $^O on the bot? (print $^O; in perl)

(In reply to comment #7) > I tried to disable NRWT on qt-mac platform: > http://trac.webkit.org/changeset/90746 > > isDarwin() checks $^O, and it should be "darwin" on the Mac bot. > Dihan, could you check what is $^O on the bot? (print $^O; in perl) It is indeed "darwin".

(In reply to comment #8) > (In reply to comment #7) > > I tried to disable NRWT on qt-mac platform: > > http://trac.webkit.org/changeset/90746 > > > > isDarwin() checks $^O, and it should be "darwin" on the Mac bot. > > Dihan, could you check what is $^O on the bot? (print $^O; in perl) > > It is indeed "darwin". The problem was that isSnowLeopard() is true for qt-mac too. Fix landed in http://trac.webkit.org/changeset/90752.

This is super-easy to fix. http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py#L66 Is the FIXME in question. This is a 3 line python change. Tell me where your httpd file is on all platforms you care about and I can do so. (Or I can just copy the logic from ORWT). Is this the only thing blocking NRWT no Qt?

I have a patch to fix this. I will post it this afternoon.

Created attachment 100378 [details] Patch

Comment on attachment 100378 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100378&action=review Thanks for fixing this, Eric. > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:396 > + if sys_platform.startswith('linux'): > + if self._is_redhat_based(): > + return 'fedora-httpd.conf' > + # FIXME: Seems wrong to assume that all non-redhat linux is debian. > + return 'apache2-debian-httpd.conf' > + if using_apache2: > + return "apache2-httpd.conf" This seems wrong to me, too. ORWT checks for the /etc/debian_version file which all debian based distros have, and apache2-httpd.conf should be used if nor Fedora nor Debian was true and the version string contained "Apache/2" since for example on Slackware and Arch the apache2 binary is called httpd. All others should fallback to httpd.conf. This would mimic ORWT behaviour.

the patch looked fine to me apart from the apache2 on mac issue ...

Created attachment 100395 [details] Patch

Comment on attachment 100395 [details] Patch I'm not an expert on this code, but this looks fine.

Comment on attachment 100395 [details] Patch Clearing flags on attachment: 100395 Committed r90810: <http://trac.webkit.org/changeset/90810>

All reviewed patches have been landed. Closing bug.