webkit-patch rebaseline-expectations should take a --platform arg
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2012 19:26:36 +0000 (19:26 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2012 19:26:36 +0000 (19:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97621

Reviewed by Ojan Vafai.

So that we can limit which platforms we attempt to rebaseline;
this can reduce the noise the command produces if some ports
have errors or warnings in their TestExpectations files and/or
help produce more predictable results.

In testing this patch, I realized that the rebaseline-in-parallel
commands (rebaseline-json, rebaseline-expectations, etc.) can
cause multiple rebaseline-test-internal commands to attempt to
modify the TestExpectations files concurrently, and that we needed
to lock the files to prevent corruption; it would be nice if
we can split the updating-the-expectations-files out separately
from updating the filesystem (much like we do with the scm updates)
to avoid this concurrency.

* Scripts/webkitpy/common/system/file_lock_mock.py: Added.
(MockFileLock):
(MockFileLock.__init__):
(MockFileLock.acquire_lock):
(MockFileLock.release_lock):
* Scripts/webkitpy/common/system/systemhost.py:
(SystemHost.copy_current_environment):
(SystemHost):
(SystemHost.make_file_lock):
* Scripts/webkitpy/common/system/systemhost_mock.py:
(MockSystemHost.copy_current_environment):
(MockSystemHost):
(MockSystemHost.make_file_lock):
* Scripts/webkitpy/tool/commands/rebaseline.py:
(RebaselineTest._update_expectations_file):
(RebaselineExpectations.__init__):
(RebaselineExpectations._add_tests_to_rebaseline_for_port):
(RebaselineExpectations.execute):

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

Tools/ChangeLog
Tools/Scripts/webkitpy/common/system/file_lock_mock.py [new file with mode: 0644]
Tools/Scripts/webkitpy/common/system/systemhost.py
Tools/Scripts/webkitpy/common/system/systemhost_mock.py
Tools/Scripts/webkitpy/tool/commands/rebaseline.py

index 7b7ee9c..fded788 100644 (file)
@@ -1,5 +1,45 @@
 2012-11-05  Dirk Pranke  <dpranke@chromium.org>
 
+        webkit-patch rebaseline-expectations should take a --platform arg
+        https://bugs.webkit.org/show_bug.cgi?id=97621
+
+        Reviewed by Ojan Vafai.
+
+        So that we can limit which platforms we attempt to rebaseline;
+        this can reduce the noise the command produces if some ports
+        have errors or warnings in their TestExpectations files and/or
+        help produce more predictable results.
+
+        In testing this patch, I realized that the rebaseline-in-parallel
+        commands (rebaseline-json, rebaseline-expectations, etc.) can
+        cause multiple rebaseline-test-internal commands to attempt to
+        modify the TestExpectations files concurrently, and that we needed
+        to lock the files to prevent corruption; it would be nice if
+        we can split the updating-the-expectations-files out separately
+        from updating the filesystem (much like we do with the scm updates)
+        to avoid this concurrency.
+
+        * Scripts/webkitpy/common/system/file_lock_mock.py: Added.
+        (MockFileLock):
+        (MockFileLock.__init__):
+        (MockFileLock.acquire_lock):
+        (MockFileLock.release_lock):
+        * Scripts/webkitpy/common/system/systemhost.py:
+        (SystemHost.copy_current_environment):
+        (SystemHost):
+        (SystemHost.make_file_lock):
+        * Scripts/webkitpy/common/system/systemhost_mock.py:
+        (MockSystemHost.copy_current_environment):
+        (MockSystemHost):
+        (MockSystemHost.make_file_lock):
+        * Scripts/webkitpy/tool/commands/rebaseline.py:
+        (RebaselineTest._update_expectations_file):
+        (RebaselineExpectations.__init__):
+        (RebaselineExpectations._add_tests_to_rebaseline_for_port):
+        (RebaselineExpectations.execute):
+
+2012-11-05  Dirk Pranke  <dpranke@chromium.org>
+
         webkitpy: clean up options for specifying multiple platforms at once
         https://bugs.webkit.org/show_bug.cgi?id=101140
 
diff --git a/Tools/Scripts/webkitpy/common/system/file_lock_mock.py b/Tools/Scripts/webkitpy/common/system/file_lock_mock.py
new file mode 100644 (file)
index 0000000..e2c1d5c
--- /dev/null
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
+# Copyright (c) 2012 Google Inc. All rights reserved.
+#
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. 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.
+#
+# THIS SOFTWARE IS PROVIDED BY UNIVERSITY OF SZEGED ``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 UNIVERSITY OF SZEGED 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.
+
+
+class MockFileLock(object):
+    def __init__(self, lock_file_path, max_wait_time_sec=20):
+        pass
+
+    def acquire_lock(self):
+        pass
+
+    def release_lock(self):
+        pass
index 3b4439e..dfec68b 100644 (file)
@@ -30,7 +30,7 @@ import os
 import platform
 import sys
 
-from webkitpy.common.system import environment, executive, filesystem, platforminfo, user, workspace
+from webkitpy.common.system import environment, executive, file_lock, filesystem, platforminfo, user, workspace
 
 
 class SystemHost(object):
@@ -43,3 +43,6 @@ class SystemHost(object):
 
     def copy_current_environment(self):
         return environment.Environment(os.environ.copy())
+
+    def make_file_lock(self, path):
+        return file_lock.FileLock(path)
index 4667b08..a529f34 100644 (file)
@@ -29,6 +29,7 @@
 from webkitpy.common.system.environment import Environment
 from webkitpy.common.system.executive_mock import MockExecutive
 from webkitpy.common.system.filesystem_mock import MockFileSystem
+from webkitpy.common.system.file_lock_mock import MockFileLock
 from webkitpy.common.system.platforminfo_mock import MockPlatformInfo
 from webkitpy.common.system.user_mock import MockUser
 from webkitpy.common.system.workspace_mock import MockWorkspace
@@ -50,3 +51,6 @@ class MockSystemHost(object):
 
     def copy_current_environment(self):
         return Environment({"MOCK_ENVIRON_COPY": '1'})
+
+    def make_file_lock(self, path):
+        return MockFileLock(path)
index e162e73..daedbc2 100644 (file)
@@ -150,13 +150,24 @@ class RebaselineTest(AbstractRebaseliningCommand):
 
     def _update_expectations_file(self, builder_name, test_name):
         port = self._tool.port_factory.get_from_builder_name(builder_name)
-        expectations = TestExpectations(port, include_overrides=False)
 
-        for test_configuration in port.all_test_configurations():
-            if test_configuration.version == port.test_configuration().version:
-                expectationsString = expectations.remove_configuration_from_test(test_name, test_configuration)
-
-        self._tool.filesystem.write_text_file(port.path_to_test_expectations_file(), expectationsString)
+        # Since rebaseline-test-internal can be called multiple times in parallel,
+        # we need to ensure that we're not trying to update the expectations file
+        # concurrently as well.
+        # FIXME: We should rework the code to not need this; maybe just download
+        # the files in parallel and rebaseline local files serially?
+        try:
+            path = port.path_to_test_expectations_file()
+            lock = self._tool.make_file_lock(path + '.lock')
+            lock.acquire_lock()
+            expectations = TestExpectations(port, include_overrides=False)
+            for test_configuration in port.all_test_configurations():
+                if test_configuration.version == port.test_configuration().version:
+                    expectationsString = expectations.remove_configuration_from_test(test_name, test_configuration)
+
+            self._tool.filesystem.write_text_file(path, expectationsString)
+        finally:
+            lock.release_lock()
 
     def _test_root(self, test_name):
         return os.path.splitext(test_name)[0]
@@ -368,11 +379,10 @@ class RebaselineExpectations(AbstractParallelRebaselineCommand):
     help_text = "Rebaselines the tests indicated in TestExpectations."
 
     def __init__(self):
-        # FIXME: We should also support platform_options here so that we only look at some TestExpectations files instead of all of them.
-        return super(RebaselineExpectations, self).__init__(options=[
+        super(RebaselineExpectations, self).__init__(options=[
             self.move_overwritten_baselines_option,
             self.no_optimize_option,
-            ])
+            ] + self.platform_options)
 
     def _update_expectations_files(self, port_name):
         port = self._tool.port_factory.get(port_name)
@@ -396,7 +406,7 @@ class RebaselineExpectations(AbstractParallelRebaselineCommand):
         tests = self._tests_to_rebaseline(self._tool.port_factory.get(port_name)).items()
 
         if tests:
-            _log.debug("Retrieving results for %s from %s." % (port_name, builder_name))
+            _log.info("Retrieving results for %s from %s." % (port_name, builder_name))
 
         for test_name, suffixes in tests:
             _log.info("    %s (%s)" % (test_name, ','.join(suffixes)))
@@ -407,7 +417,8 @@ class RebaselineExpectations(AbstractParallelRebaselineCommand):
     def execute(self, options, args, tool):
         options.results_directory = None
         self._test_list = {}
-        for port_name in tool.port_factory.all_port_names():
+        port_names = tool.port_factory.all_port_names(options.platform)
+        for port_name in port_names:
             self._add_tests_to_rebaseline_for_port(port_name)
         if not self._test_list:
             _log.warning("Did not find any tests marked Rebaseline.")
@@ -415,7 +426,7 @@ class RebaselineExpectations(AbstractParallelRebaselineCommand):
 
         self._rebaseline(options, self._test_list)
 
-        for port_name in tool.port_factory.all_port_names():
+        for port_name in port_names:
             self._update_expectations_files(port_name)