Bug 64086 - [Qt] NRWT should pick up the right httpd config file
: [Qt] NRWT should pick up the right httpd config file
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: Qt, QtTriaged
: 64441
: 34984
  Show dependency treegraph
 
Reported: 2011-07-07 06:57 PST by
Modified: 2011-07-13 02:36 PST (History)


Attachments
proposed fix (1.69 KB, patch)
2011-07-07 06:59 PST, Andras Becsi
eric: review-
Review Patch | Details | Formatted Diff | Diff
Patch (17.67 KB, patch)
2011-07-11 16:28 PST, Eric Seidel
no flags Review Patch | Details | Formatted Diff | Diff
Patch (46.51 KB, patch)
2011-07-11 17:59 PST, Eric Seidel
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-07-07 06:57:33 PST
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.
------- Comment #1 From 2011-07-07 06:59:42 PST -------
Created an attachment (id=99977) [details]
proposed fix
------- Comment #2 From 2011-07-07 10:10:37 PST -------
(From update of attachment 99977 [details])
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 #3 From 2011-07-07 10:11:39 PST -------
(From update of attachment 99977 [details])
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.)
------- Comment #4 From 2011-07-11 04:23:35 PST -------
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
------- Comment #5 From 2011-07-11 04:26:59 PST -------
Assigning this bug to Kristóf, since he has more knowledge of python than I have.
------- Comment #6 From 2011-07-11 04:28:03 PST -------
(From update of attachment 99977 [details])
Marking my patch obsolete.
------- Comment #7 From 2011-07-11 08:40:59 PST -------
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)
------- Comment #8 From 2011-07-11 08:59:23 PST -------
(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".
------- Comment #9 From 2011-07-11 09:00:43 PST -------
(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.
------- Comment #10 From 2011-07-11 10:50:48 PST -------
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?
------- Comment #11 From 2011-07-11 12:42:56 PST -------
I have a patch to fix this.  I will post it this afternoon.
------- Comment #12 From 2011-07-11 16:28:05 PST -------
Created an attachment (id=100378) [details]
Patch
------- Comment #13 From 2011-07-11 17:06:38 PST -------
(From update of attachment 100378 [details])
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.
------- Comment #14 From 2011-07-11 17:17:07 PST -------
the patch looked fine to me apart from the apache2 on mac issue ...
------- Comment #15 From 2011-07-11 17:59:38 PST -------
Created an attachment (id=100395) [details]
Patch
------- Comment #16 From 2011-07-12 00:27:01 PST -------
(From update of attachment 100395 [details])
I'm not an expert on this code, but this looks fine.
------- Comment #17 From 2011-07-12 01:08:44 PST -------
(From update of attachment 100395 [details])
Clearing flags on attachment: 100395

Committed r90810: <http://trac.webkit.org/changeset/90810>
------- Comment #18 From 2011-07-12 01:08:50 PST -------
All reviewed patches have been landed.  Closing bug.