Bug 37566 – new-run-webkit-tests: code outside of port/ should never use sys.platform

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

code outside of port/factory.py should never use sys.platform

% grep "sys.platform" -r *
layout_tests/layout_package/json_results_generator.py:                                      shell=(sys.platform == 'win32'),
layout_tests/port/apache_http_server.py:        return sys.platform in ("win32", "cygwin")
layout_tests/port/base.py:        if sys.platform in ('cygwin', 'win32'):
layout_tests/port/base.py:        if sys.platform == 'win32':
layout_tests/port/chromium.py:        close_flag = sys.platform not in ('win32', 'cygwin')
layout_tests/port/chromium.py:            if sys.platform not in ('win32', 'cygwin'):
layout_tests/port/factory.py:        if sys.platform == 'win32' or sys.platform == 'cygwin':
layout_tests/port/factory.py:        elif sys.platform == 'linux2':
layout_tests/port/factory.py:        elif sys.platform == 'darwin':
layout_tests/port/factory.py:        raise NotImplementedError('unknown port; sys.platform = "%s"' %
layout_tests/port/factory.py:                                  sys.platform)
layout_tests/port/http_server.py:        if sys.platform == 'darwin':
layout_tests/port/http_server.py:        if sys.platform in ('cygwin', 'win32'):
layout_tests/port/http_server.py:        if sys.platform == 'win32' and self._register_cygwin:
layout_tests/port/server_process.py:        close_fds = sys.platform not in ('win32', 'cygwin')
layout_tests/port/server_process.py:        if sys.platform not in ('win32', 'cygwin'):
layout_tests/port/websocket_server.py:        if sys.platform in ('cygwin', 'win32'):
layout_tests/port/websocket_server.py:        if sys.platform == 'win32' and self._register_cygwin:
layout_tests/rebaseline_chromium_webkit_tests.py:    use_shell = sys.platform.startswith('win')
layout_tests/rebaseline_chromium_webkit_tests.py:            if sys.platform.startswith("win"):
layout_tests/run_webkit_tests.py:        while ((directory != 'http' or sys.platform in ('darwin', 'linux2'))
layout_tests/run_webkit_tests.py:        options.use_apache = sys.platform in ('darwin', 'linux2')

Pretty much every one of those is a bug/layering violation.

I'll look at that list more this week.

[Dirk Pranke]

The original design theory of the port package was that all platform-specific functionality was hidden below it, and the code above it would be clean. So, I agree that code in run_webkit_tests, layout_package, etc. should not read sys.platform.

I feel less strongly about code inside the port/ directory. I think in general it's a good idea to avoid reading sys.platform to implement large branches in the code, but the maintenance impact of trying to stuff every branch of the code into separate files can make maintenance harder in some cases (i.e., I don't think it needs to be a black and white rule).

[Eric Seidel (no email)]

Yes, I agree.  I was inexact in my grepping. :)

[Dirk Pranke]

(In reply to comment #2)
> Yes, I agree.  I was inexact in my grepping. :)

Perhaps change the subject line then? Maybe "code outside of port/ must not use sys.platform" or something?

[Dirk Pranke]

closing, except for bug 89616, all the remaining uses are either in test code to skip tests on windows, or in port/ or servers/ to handle actually port-specific code.