Changeset 51969 in webkit

Timestamp:

Dec 10, 2009 3:47:36 PM (14 years ago)

Author:

eric@webkit.org

Message:

2009-12-10 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

Move run_command onto Executive to make code which uses run_command testable
​https://bugs.webkit.org/show_bug.cgi?id=32396

• Scripts/modules/executive.py:
  • Move run_command and error handlers onto Executive.
• Scripts/modules/scm.py:
• Scripts/modules/scm_unittest.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/modules/executive.py (modified) (2 diffs)
• Scripts/modules/scm.py (modified) (5 diffs)
• Scripts/modules/scm_unittest.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-12-10  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        Move run_command onto Executive to make code which uses run_command testable
+        https://bugs.webkit.org/show_bug.cgi?id=32396
+
+        * Scripts/modules/executive.py:
+         - Move run_command and error handlers onto Executive.
+        * Scripts/modules/scm.py:
+        * Scripts/modules/scm_unittest.py:
+
 2009-12-09  Eric Seidel  <eric@webkit.org>
 

trunk/WebKitTools/Scripts/modules/executive.py

@@ -58 +58 @@
         return str(self)
 
-def default_error_handler(error):
-    raise error
-
-def ignore_error(error):
-    pass
 
 # FIXME: This should not be a global static.
-def run_command(args, cwd=None, input=None, error_handler=default_error_handler, return_exit_code=False, return_stderr=True):
-    if hasattr(input, 'read'): # Check if the input is a file.
-        stdin = input
-        string_to_communicate = None
-    else:
-        stdin = subprocess.PIPE if input else None
-        string_to_communicate = input
-    if return_stderr:
-        stderr = subprocess.STDOUT
-    else:
-        stderr = None
-    process = subprocess.Popen(args, stdin=stdin, stdout=subprocess.PIPE, stderr=stderr, cwd=cwd)
-    output = process.communicate(string_to_communicate)[0]
-    exit_code = process.wait()
-    if exit_code:
-        script_error = ScriptError(script_args=args, exit_code=exit_code, output=output, cwd=cwd)
-        error_handler(script_error)
-    if return_exit_code:
-        return exit_code
-    return output
+# New code should use Executive.run_command directly instead
+def run_command(*args, **kwargs):
+    return Executive().run_command(*args, **kwargs)
 
 
@@ -114 +92 @@
         if exit_code:
             raise ScriptError(script_args=args, exit_code=exit_code, output=child_output)
+
+    # Error handlers do not need to be static methods once all callers are updated to use an Executive object.
+    @staticmethod
+    def default_error_handler(error):
+        raise error
+
+    @staticmethod
+    def ignore_error(error):
+        pass
+
+    # FIXME: This should be merged with run_and_throw_if_fail
+    def run_command(self, args, cwd=None, input=None, error_handler=None, return_exit_code=False, return_stderr=True):
+        if hasattr(input, 'read'): # Check if the input is a file.
+            stdin = input
+            string_to_communicate = None
+        else:
+            stdin = subprocess.PIPE if input else None
+            string_to_communicate = input
+        if return_stderr:
+            stderr = subprocess.STDOUT
+        else:
+            stderr = None
+        process = subprocess.Popen(args, stdin=stdin, stdout=subprocess.PIPE, stderr=stderr, cwd=cwd)
+        output = process.communicate(string_to_communicate)[0]
+        exit_code = process.wait()
+        if exit_code:
+            script_error = ScriptError(script_args=args, exit_code=exit_code, output=output, cwd=cwd)
+            (error_handler or self.default_error_handler)(script_error)
+        if return_exit_code:
+            return exit_code
+        return output

trunk/WebKitTools/Scripts/modules/scm.py

@@ -36 +36 @@
 # Import WebKit-specific modules.
 from modules.changelogs import ChangeLog
-from modules.executive import run_command, ScriptError, default_error_handler, ignore_error
+from modules.executive import Executive, run_command, ScriptError
 from modules.logging import error, log
 
@@ -88 +88 @@
     if re.search("resource out of date", error.output):
         raise CheckoutNeedsUpdate(script_args=error.script_args, exit_code=error.exit_code, output=error.output, cwd=error.cwd)
-    default_error_handler(error)
+    Executive.default_error_handler(error)
 
 
@@ -105 +105 @@
     def ensure_clean_working_directory(self, force_clean):
         if not force_clean and not self.working_directory_is_clean():
-            print run_command(self.status_command(), error_handler=ignore_error)
+            print run_command(self.status_command(), error_handler=Executive.ignore_error)
             raise ScriptError(message="Working directory has modifications, pass --force-clean or --no-clean to continue.")
         
@@ -369 +369 @@
     @classmethod
     def in_working_directory(cls, path):
-        return run_command(['git', 'rev-parse', '--is-inside-work-tree'], cwd=path, error_handler=ignore_error).rstrip() == "true"
+        return run_command(['git', 'rev-parse', '--is-inside-work-tree'], cwd=path, error_handler=Executive.ignore_error).rstrip() == "true"
 
     @classmethod
@@ -439 +439 @@
         # I think this will always fail due to ChangeLogs.
         # FIXME: We need to detec specific failure conditions and handle them.
-        run_command(['git', 'revert', '--no-commit', git_commit], error_handler=ignore_error)
+        run_command(['git', 'revert', '--no-commit', git_commit], error_handler=Executive.ignore_error)
 
         # Fix any ChangeLogs if necessary.

trunk/WebKitTools/Scripts/modules/scm_unittest.py

@@ -39 +39 @@
 
 from datetime import date
-from modules.executive import run_command, ignore_error, ScriptError
+from modules.executive import Executive, run_command, ScriptError
 from modules.scm import detect_scm_system, SCM, CheckoutNeedsUpdate, commit_error_handler
 
@@ -148 +148 @@
         command_does_not_exist = ['does_not_exist', 'invalid_option']
         self.assertRaises(OSError, run_command, command_does_not_exist)
-        self.assertRaises(OSError, run_command, command_does_not_exist, error_handler=ignore_error)
+        self.assertRaises(OSError, run_command, command_does_not_exist, error_handler=Executive.ignore_error)
 
         command_returns_non_zero = ['/bin/sh', '--invalid-option']
         self.assertRaises(ScriptError, run_command, command_returns_non_zero)
         # Check if returns error text:
-        self.assertTrue(run_command(command_returns_non_zero, error_handler=ignore_error))
+        self.assertTrue(run_command(command_returns_non_zero, error_handler=Executive.ignore_error))
 
         self.assertRaises(CheckoutNeedsUpdate, commit_error_handler, ScriptError(output=git_failure_message))