2010-11-11 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Nov 2010 23:51:00 +0000 (23:51 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Nov 2010 23:51:00 +0000 (23:51 +0000)
        Reviewed by Adam Roben.

        Fix NRWT to respect set-webkit-configuration again :(

        This change fixes a typo in config.py that was causing the wrong
        value to be read initially and us never actually looking into
        the filesystem to get the default configuration.

        * Scripts/webkitpy/layout_tests/port/config.py:
        * Scripts/webkitpy/layout_tests/port/config_standalone.py:
        * Scripts/webkitpy/layout_tests/port/config_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71858 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/port/config.py
WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/port/config_unittest.py

index 977265d1f04e828d7ffe2c159cc629ea936738e2..7c8c5daf777c35e64a4f04c4e95e06ab22a45601 100644 (file)
@@ -1,3 +1,17 @@
+2010-11-11  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Adam Roben.
+
+        Fix NRWT to respect set-webkit-configuration again :(
+
+        This change fixes a typo in config.py that was causing the wrong
+        value to be read initially and us never actually looking into
+        the filesystem to get the default configuration.
+        
+        * Scripts/webkitpy/layout_tests/port/config.py:
+        * Scripts/webkitpy/layout_tests/port/config_standalone.py:
+        * Scripts/webkitpy/layout_tests/port/config_unittest.py:
+
 2010-11-11  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Adam Barth.
index 4a0de96346ab4cab8706cdf79ce25b1d17e582fb..1d7f4fee6095be161256148884af86815a1f9cac 100644 (file)
@@ -40,16 +40,20 @@ from webkitpy.common.system import logutils
 _log = logutils.get_logger(__file__)
 
 #
-# This is used to record if we've already hit the filesystem to look
+# FIXME: This is used to record if we've already hit the filesystem to look
 # for a default configuration. We cache this to speed up the unit tests,
-# but this can be reset with clear_cached_configuration().
+# but this can be reset with clear_cached_configuration(). This should be
+# replaced with us consistently using MockConfigs() for tests that don't
+# hit the filesystem at all and provide a reliable value.
 #
-_determined_configuration = None
+_have_determined_configuration = False
+_configuration = "Release"
 
 
 def clear_cached_configuration():
-    global _determined_configuration
-    _determined_configuration = -1
+    global _have_determined_configuration, _configuration
+    _have_determined_configuration = False
+    _configuration = "Release"
 
 
 class Config(object):
@@ -137,8 +141,11 @@ class Config(object):
 
     def _determine_configuration(self):
         # This mirrors the logic in webkitdirs.pm:determineConfiguration().
-        global _determined_configuration
-        if _determined_configuration == -1:
+        #
+        # FIXME: See the comment at the top of the file regarding unit tests
+        # and our use of global mutable static variables.
+        global _have_determined_configuration, _configuration
+        if not _have_determined_configuration:
             contents = self._read_configuration()
             if not contents:
                 contents = "Release"
@@ -146,8 +153,9 @@ class Config(object):
                 contents = "Release"
             if contents == "Development":
                 contents = "Debug"
-            _determined_configuration = contents
-        return _determined_configuration
+            _configuration = contents
+            _have_determined_configuration = True
+        return _configuration
 
     def _read_configuration(self):
         configuration_path = self._filesystem.join(self.build_directory(None),
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py
new file mode 100644 (file)
index 0000000..1fc81db
--- /dev/null
@@ -0,0 +1,62 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#    * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#    * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#    * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""FIXME: This script is used by
+config_unittest.test_default_configuration__standalone() to read the
+default configuration to work around any possible caching / reset bugs. See
+https://bugs.webkit.org/show_bug?id=49360 for the motivation. We can remove
+this test when we remove the global configuration cache in config.py."""
+
+import os
+import unittest
+
+from webkitpy.common.system import executive
+from webkitpy.common.system import executive_mock
+from webkitpy.common.system import filesystem
+from webkitpy.common.system import filesystem_mock
+
+import sys
+import config
+
+
+def main(argv=None):
+    if not argv:
+        argv = sys.argv
+
+    if len(argv) == 3 and argv[1] == '--mock':
+        e = executive_mock.MockExecutive2(output='foo')
+        fs = filesystem_mock.MockFileSystem({'foo/Configuration': argv[2]})
+    else:
+        e = executive.Executive()
+        fs = filesystem.FileSystem()
+
+    c = config.Config(e, fs)
+    print c.default_configuration()
+
+if __name__ == '__main__':
+    main()
index ed5ab8b7304e05dea47198f9e4472fcc580d65eb..f5e2d3a35d14ee9e62d5a916edfe2168f84f26a7 100644 (file)
@@ -27,6 +27,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import os
+import sys
 import unittest
 
 from webkitpy.common.system import executive
@@ -48,7 +49,7 @@ class ConfigTest(unittest.TestCase):
 
     def assert_configuration(self, contents, expected):
         # This tests that a configuration file containing
-        # _contents_ endsd up being interpreted as _expected_.
+        # _contents_ ends up being interpreted as _expected_.
         c = self.make_config('foo', {'foo/Configuration': contents})
         self.assertEqual(c.default_configuration(), expected)
 
@@ -119,6 +120,25 @@ class ConfigTest(unittest.TestCase):
         self.assert_configuration('Unknown', 'Unknown')
         oc.restore_output()
 
+    def test_default_configuration__standalone(self):
+        # FIXME: This test runs a standalone python script to test
+        # reading the default configuration to work around any possible
+        # caching / reset bugs. See https://bugs.webkit.org/show_bug?id=49360
+        # for the motivation. We can remove this test when we remove the
+        # global configuration cache in config.py.
+        e = executive.Executive()
+        fs = filesystem.FileSystem()
+        c = config.Config(e, fs)
+        script = c.path_from_webkit_base('WebKitTools', 'Scripts',
+            'webkitpy', 'layout_tests', 'port', 'config_standalone.py')
+
+        # Note: don't use 'Release' here, since that's the normal default.
+        expected = 'Debug'
+
+        args = [sys.executable, script, '--mock', expected]
+        actual = e.run_command(args).rstrip()
+        self.assertEqual(actual, expected)
+
     def test_path_from_webkit_base(self):
         # FIXME: We use a real filesystem here. Should this move to a
         # mocked one?