Bug 64086

Summary: [Qt] NRWT should pick up the right httpd config file
Product: WebKit Reporter: Andras Becsi <abecsi>
Component: Tools / TestsAssignee: Kristóf Kosztyó <kkristof>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dihan.wickremasuriya, dpranke, eric, galpeter, kkristof, laszlo.gombos, ossy, rgabor, webkit.review.bot
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 64441    
Bug Blocks: 34984    
Attachments:
Description Flags
proposed fix
eric: review-
Patch
none
Patch none

Andras Becsi
Reported 2011-07-07 06:57:33 PDT
NRWT currently uses apache2-debian-httpd.conf for running the httpd server, thus only runs on debian based distros, this is a huge regression compared to ORWT. NRWT should at least check for the /etc/debian_version file, and if not present it should use the generic apache2-httpd.conf file.
Attachments
proposed fix (1.69 KB, patch)
2011-07-07 06:59 PDT, Andras Becsi
eric: review-
Patch (17.67 KB, patch)
2011-07-11 16:28 PDT, Eric Seidel (no email)
no flags
Patch (46.51 KB, patch)
2011-07-11 17:59 PDT, Eric Seidel (no email)
no flags
Andras Becsi
Comment 1 2011-07-07 06:59:42 PDT
Created attachment 99977 [details] proposed fix
Eric Seidel (no email)
Comment 2 2011-07-07 10:10:37 PDT
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.
Eric Seidel (no email)
Comment 3 2011-07-07 10:11:39 PDT
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.)
Csaba Osztrogonác
Comment 4 2011-07-11 04:23:35 PDT
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
Andras Becsi
Comment 5 2011-07-11 04:26:59 PDT
Assigning this bug to Kristóf, since he has more knowledge of python than I have.
Andras Becsi
Comment 6 2011-07-11 04:28:03 PDT
Comment on attachment 99977 [details] proposed fix Marking my patch obsolete.
Csaba Osztrogonác
Comment 7 2011-07-11 08:40:59 PDT
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)
Dihan Wickremasuriya
Comment 8 2011-07-11 08:59:23 PDT
(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".
Csaba Osztrogonác
Comment 9 2011-07-11 09:00:43 PDT
(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.
Eric Seidel (no email)
Comment 10 2011-07-11 10:50:48 PDT
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?
Eric Seidel (no email)
Comment 11 2011-07-11 12:42:56 PDT
I have a patch to fix this. I will post it this afternoon.
Eric Seidel (no email)
Comment 12 2011-07-11 16:28:05 PDT
Andras Becsi
Comment 13 2011-07-11 17:06:38 PDT
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.
Dirk Pranke
Comment 14 2011-07-11 17:17:07 PDT
the patch looked fine to me apart from the apache2 on mac issue ...
Eric Seidel (no email)
Comment 15 2011-07-11 17:59:38 PDT
Adam Barth
Comment 16 2011-07-12 00:27:01 PDT
Comment on attachment 100395 [details] Patch I'm not an expert on this code, but this looks fine.
WebKit Review Bot
Comment 17 2011-07-12 01:08:44 PDT
Comment on attachment 100395 [details] Patch Clearing flags on attachment: 100395 Committed r90810: <http://trac.webkit.org/changeset/90810>
WebKit Review Bot
Comment 18 2011-07-12 01:08:50 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.