Check style on test_expectations.txt files before commit

https://bugs.webkit.org/show_bug.cgi?id=76484

Reviewed by Adam Barth.

Unlike other style rules, if you get the syntax of the test_expectations.txt
file wrong, the layout tests won't run. Also, this check is simple and only
slows down committing if you actually modify one of the test_expectations.txt files.

* Scripts/webkitpy/tool/steps/commit.py:
(Commit._check_test_expectations):
(Commit.run):
* Scripts/webkitpy/tool/steps/commit_unittest.py: Added.
(CommitTest):
(CommitTest.test_check_test_expectations):

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

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 11cd4d5..c9136c3 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,21 @@
+2012-01-17  Ojan Vafai  <ojan@chromium.org>
+
+        Check style on test_expectations.txt files before commit
+        https://bugs.webkit.org/show_bug.cgi?id=76484
+
+        Reviewed by Adam Barth.
+
+        Unlike other style rules, if you get the syntax of the test_expectations.txt
+        file wrong, the layout tests won't run. Also, this check is simple and only
+        slows down committing if you actually modify one of the test_expectations.txt files.
+
+        * Scripts/webkitpy/tool/steps/commit.py:
+        (Commit._check_test_expectations):
+        (Commit.run):
+        * Scripts/webkitpy/tool/steps/commit_unittest.py: Added.
+        (CommitTest):
+        (CommitTest.test_check_test_expectations):
+
 2012-01-17  Viatcheslav Ostapenko  <ostapenko.viatcheslav@nokia.com>
 
         [Qt] Debug build fails with debug qt5

diff --git a/Tools/Scripts/webkitpy/tool/steps/commit.py b/Tools/Scripts/webkitpy/tool/steps/commit.py
index 08a7310..e8fc392 100644 (file)
--- a/Tools/Scripts/webkitpy/tool/steps/commit.py
+++ b/Tools/Scripts/webkitpy/tool/steps/commit.py
@@ -50,11 +50,30 @@ class Commit(AbstractStep):
                 'To avoid this prompt, set "git config webkit-patch.commit-should-always-squash true".' % (
                 error.num_local_commits, working_directory_message))
 
+    def _check_test_expectations(self, changed_files):
+        test_expectations_files = []
+        for filename in changed_files:
+            if filename.endswith('test_expectations.txt'):
+                test_expectations_files.append(filename)
+
+        if not test_expectations_files:
+            return
+
+        args = ["--diff-files"]
+        args.extend(test_expectations_files)
+        try:
+            self._tool.executive.run_and_throw_if_fail(self._tool.port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root)
+        except ScriptError, e:
+            if not self._tool.user.confirm("Are you sure you want to continue?", default="n"):
+                exit(1)
+
     def run(self, state):
         self._commit_message = self._tool.checkout().commit_message_for_this_commit(self._options.git_commit).message()
         if len(self._commit_message) < 10:
             raise Exception("Attempted to commit with a commit message shorter than 10 characters.  Either your patch is missing a ChangeLog or webkit-patch may have a bug.")
 
+        self._check_test_expectations(self._changed_files(state))
+
         self._state = state
 
         username = None

diff --git a/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py b/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py
new file mode 100644 (file)
index 0000000..b0e6fd1
--- /dev/null
+++ b/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2012 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.
+
+import os
+import unittest
+
+from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.common.system.executive_mock import MockExecutive
+from webkitpy.tool.mocktool import MockOptions, MockTool
+from webkitpy.tool.steps.commit import Commit
+
+
+class CommitTest(unittest.TestCase):
+    def test_check_test_expectations(self):
+        capture = OutputCapture()
+        options = MockOptions()
+        options.git_commit = ""
+
+        tool = MockTool()
+        step = Commit(tool, options)
+        state = {
+            "changed_files": ["test_expectations.txtXXX"],
+        }
+
+        tool.executive = MockExecutive(should_log=True, should_throw_when_run=False)
+        capture.assert_outputs(self, step.run, [state], expected_stderr="Committed r49824: <http://trac.webkit.org/changeset/49824>\n")
+
+        state = {
+            "changed_files": ["platform/chromium/test_expectations.txt"],
+        }
+        capture.assert_outputs(self, step.run, [state], expected_stderr="MOCK run_and_throw_if_fail: ['mock-check-webkit-style', '--diff-files', 'platform/chromium/test_expectations.txt'], cwd=/mock-checkout\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\n")
+
+        tool.executive = MockExecutive(should_log=True, should_throw_when_run=set(["platform/chromium/test_expectations.txt"]))
+        self.assertRaises(SystemExit, capture.assert_outputs, self, step.run, [state])