Snap for 11647390 from e106ee3301113116bdc4e11cdb9af60ea946d12b to androidx-vectordrawable-releaseandroidx-vectordrawable-release

Change-Id: I234e589b2a5d1f393da3a20e19919641bbd869b7

6 files changed, 361 insertions, 131 deletions

diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 8ee46ac..31de3b0 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -2,6 +2,7 @@
 # Only list fast unittests here.
 config_unittest = ./rh/config_unittest.py
 hooks_unittest  = ./rh/hooks_unittest.py
+results_unittest = ./rh/results_unittest.py
 shell_unittest  = ./rh/shell_unittest.py
 terminal_unittest  = ./rh/terminal_unittest.py
 utils_unittest  = ./rh/utils_unittest.py
diff --git a/pre-upload.py b/pre-upload.py
index 5c820c6..18bf11f 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -20,6 +20,7 @@ when developing.
 """
 
 import argparse
+import concurrent.futures
 import datetime
 import os
 import signal
@@ -63,6 +64,7 @@ class Output(object):
     PASSED = COLOR.color(COLOR.GREEN, 'PASSED')
     FAILED = COLOR.color(COLOR.RED, 'FAILED')
     WARNING = COLOR.color(COLOR.YELLOW, 'WARNING')
+    FIXUP = COLOR.color(COLOR.MAGENTA, 'FIXUP')
 
     # How long a hook is allowed to run before we warn that it is "too slow".
     _SLOW_HOOK_DURATION = datetime.timedelta(seconds=30)
@@ -74,22 +76,15 @@ class Output(object):
           project_name: name of project.
         """
         self.project_name = project_name
+        self.hooks = None
         self.num_hooks = None
-        self.hook_index = 0
         self.num_commits = None
         self.commit_index = 0
         self.success = True
         self.start_time = datetime.datetime.now()
         self.hook_start_time = None
-        self._curr_hook_name = None
-
-    def set_num_hooks(self, num_hooks):
-        """Keep track of how many hooks we'll be running.
-
-        Args:
-          num_hooks: number of hooks to be run.
-        """
-        self.num_hooks = num_hooks
+        # Cache number of invisible characters in our banner.
+        self._banner_esc_chars = len(self.COLOR.color(self.COLOR.YELLOW, ''))
 
     def set_num_commits(self, num_commits: int) -> None:
         """Keep track of how many commits we'll be running.
@@ -100,10 +95,11 @@ class Output(object):
         self.num_commits = num_commits
         self.commit_index = 1
 
-    def commit_start(self, commit, commit_summary):
+    def commit_start(self, hooks, commit, commit_summary):
         """Emit status for new commit.
 
         Args:
+          hooks: All the hooks to be run for this commit.
           commit: commit hash.
           commit_summary: commit summary.
         """
@@ -113,47 +109,58 @@ class Output(object):
             f'{commit[0:12]}] {commit_summary}'
         )
         rh.terminal.print_status_line(status_line, print_newline=True)
-        self.hook_index = 1
         self.commit_index += 1
 
-    def hook_start(self, hook_name):
-        """Emit status before the start of a hook.
+        # Initialize the pending hooks line too.
+        self.hooks = set(hooks)
+        self.num_hooks = len(hooks)
+        self.hook_banner()
 
-        Args:
-          hook_name: name of the hook.
-        """
-        self._curr_hook_name = hook_name
-        self.hook_start_time = datetime.datetime.now()
-        status_line = (f'[{self.RUNNING} {self.hook_index}/{self.num_hooks}] '
-                       f'{hook_name}')
-        self.hook_index += 1
+    def hook_banner(self):
+        """Display the banner for current set of hooks."""
+        pending = ', '.join(x.name for x in self.hooks)
+        status_line = (
+            f'[{self.RUNNING} '
+            f'{self.num_hooks - len(self.hooks)}/{self.num_hooks}] '
+            f'{pending}'
+        )
+        if self._banner_esc_chars and sys.stderr.isatty():
+            cols = os.get_terminal_size(sys.stderr.fileno()).columns
+            status_line = status_line[0:cols + self._banner_esc_chars]
         rh.terminal.print_status_line(status_line)
 
-    def hook_finish(self):
+    def hook_finish(self, hook, duration):
         """Finish processing any per-hook state."""
-        duration = datetime.datetime.now() - self.hook_start_time
+        self.hooks.remove(hook)
         if duration >= self._SLOW_HOOK_DURATION:
             d = rh.utils.timedelta_str(duration)
             self.hook_warning(
+                hook,
                 f'This hook took {d} to finish which is fairly slow for '
                 'developers.\nPlease consider moving the check to the '
                 'server/CI system instead.')
 
-    def hook_error(self, error):
+        # Show any hooks still pending.
+        if self.hooks:
+            self.hook_banner()
+
+    def hook_error(self, hook, error):
         """Print an error for a single hook.
 
         Args:
+          hook: The hook that generated the output.
           error: error string.
         """
-        self.error(f'{self._curr_hook_name} hook', error)
+        self.error(f'{hook.name} hook', error)
 
-    def hook_warning(self, warning):
+    def hook_warning(self, hook, warning):
         """Print a warning for a single hook.
 
         Args:
+          hook: The hook that generated the output.
           warning: warning string.
         """
-        status_line = f'[{self.WARNING}] {self._curr_hook_name}'
+        status_line = f'[{self.WARNING}] {hook.name}'
         rh.terminal.print_status_line(status_line, print_newline=True)
         print(warning, file=sys.stderr)
 
@@ -169,6 +176,21 @@ class Output(object):
         print(error, file=sys.stderr)
         self.success = False
 
+    def hook_fixups(
+        self,
+        project_results: rh.results.ProjectResults,
+        hook_results: List[rh.results.HookResult],
+    ) -> None:
+        """Display summary of possible fixups for a single hook."""
+        for result in (x for x in hook_results if x.fixup_cmd):
+            cmd = result.fixup_cmd + list(result.files)
+            for line in (
+                f'[{self.FIXUP}] {result.hook} has automated fixups available',
+                f'  cd {rh.shell.quote(project_results.workdir)} && \\',
+                f'    {rh.shell.cmd_to_str(cmd)}',
+            ):
+                rh.terminal.print_status_line(line, print_newline=True)
+
     def finish(self):
         """Print summary for all the hooks."""
         header = self.PASSED if self.success else self.FAILED
@@ -200,7 +222,7 @@ def _process_hook_results(results):
     error_ret = ''
     warning_ret = ''
     for result in results:
-        if result:
+        if result or result.is_warning():
             ret = ''
             if result.files:
                 ret += f'  FILES: {rh.shell.cmd_to_str(result.files)}\n'
@@ -242,51 +264,86 @@ def _get_project_config(from_git=False):
     return rh.config.PreUploadSettings(paths=paths, global_paths=global_paths)
 
 
-def _attempt_fixes(fixup_list, commit_list, cwd):
-    """Attempts to run |fixup_list| given |commit_list|."""
-    if len(fixup_list) != 1:
-        # Only single fixes will be attempted, since various fixes might
-        # interact with each other.
+def _attempt_fixes(projects_results: List[rh.results.ProjectResults]) -> None:
+    """Attempts to fix fixable results."""
+    # Filter out any result that has a fixup.
+    fixups = []
+    for project_results in projects_results:
+        fixups.extend((project_results.workdir, x)
+                      for x in project_results.fixups)
+    if not fixups:
         return
 
-    hook_name, commit, fixup_cmd, fixup_files = fixup_list[0]
+    if len(fixups) > 1:
+        banner = f'Multiple fixups ({len(fixups)}) are available.'
+    else:
+        banner = 'Automated fixups are available.'
+    print(Output.COLOR.color(Output.COLOR.MAGENTA, banner), file=sys.stderr)
+
+    # If there's more than one fixup available, ask if they want to blindly run
+    # them all, or prompt for them one-by-one.
+    mode = 'some'
+    if len(fixups) > 1:
+        while True:
+            response = rh.terminal.str_prompt(
+                'What would you like to do',
+                ('Run (A)ll', 'Run (S)ome', '(D)ry-run', '(N)othing [default]'))
+            if not response:
+                print('', file=sys.stderr)
+                return
+            if response.startswith('a') or response.startswith('y'):
+                mode = 'all'
+                break
+            elif response.startswith('s'):
+                mode = 'some'
+                break
+            elif response.startswith('d'):
+                mode = 'dry-run'
+                break
+            elif response.startswith('n'):
+                print('', file=sys.stderr)
+                return
+
+    # Walk all the fixups and run them one-by-one.
+    for workdir, result in fixups:
+        if mode == 'some':
+            if not rh.terminal.boolean_prompt(
+                f'Run {result.hook} fixup for {result.commit}'
+            ):
+                continue
 
-    if commit != commit_list[0]:
-        # If the commit is not at the top of the stack, git operations might be
-        # needed and might leave the working directory in a tricky state if the
-        # fix is attempted to run automatically (e.g. it might require manual
-        # merge conflict resolution). Refuse to run the fix in those cases.
-        return
+        cmd = tuple(result.fixup_cmd) + tuple(result.files)
+        print(
+            f'\n[{Output.RUNNING}] cd {rh.shell.quote(workdir)} && '
+            f'{rh.shell.cmd_to_str(cmd)}', file=sys.stderr)
+        if mode == 'dry-run':
+            continue
 
-    prompt = (f'An automatic fix can be attempted for the "{hook_name}" hook. '
-              'Do you want to run it?')
-    if not rh.terminal.boolean_prompt(prompt):
-        return
-
-    result = rh.utils.run(
-        fixup_cmd + list(fixup_files),
-        cwd=cwd,
-        combine_stdout_stderr=True,
-        capture_output=True,
-        check=False,
-        input='',
-    )
-    if result.returncode:
-        print(f'Attempt to fix "{hook_name}" for commit "{commit}" failed: '
-              f'{result.stdout}',
-              file=sys.stderr)
-    else:
-        print('Fix successfully applied. Amend the current commit before '
-              'attempting to upload again.\n', file=sys.stderr)
+        cmd_result = rh.utils.run(cmd, cwd=workdir, check=False)
+        if cmd_result.returncode:
+            print(f'[{Output.WARNING}] command exited {cmd_result.returncode}',
+                  file=sys.stderr)
+        else:
+            print(f'[{Output.PASSED}] great success', file=sys.stderr)
 
+    print(f'\n[{Output.FIXUP}] Please amend & rebase your tree before '
+          'attempting to upload again.\n', file=sys.stderr)
 
-def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, commit_list=None):
+def _run_project_hooks_in_cwd(
+    project_name: str,
+    proj_dir: str,
+    output: Output,
+    jobs: Optional[int] = None,
+    from_git: bool = False,
+    commit_list: Optional[List[str]] = None,
+) -> rh.results.ProjectResults:
     """Run the project-specific hooks in the cwd.
 
     Args:
       project_name: The name of this project.
       proj_dir: The directory for this project (for passing on in metadata).
       output: Helper for summarizing output/errors to the user.
+      jobs: How many hooks to run in parallel.
       from_git: If true, we are called from git directly and repo should not be
           used.
       commit_list: A list of commits to run hooks against.  If None or empty
@@ -294,20 +351,20 @@ def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, co
           uploaded.
 
     Returns:
-      True if everything passed, else False.
+      All the results for this project.
     """
+    ret = rh.results.ProjectResults(project_name, proj_dir)
+
     try:
         config = _get_project_config(from_git)
     except rh.config.ValidationError as e:
         output.error('Loading config files', str(e))
-        return False
+        return ret._replace(internal_failure=True)
 
     # If the repo has no pre-upload hooks enabled, then just return.
     hooks = list(config.callable_hooks())
     if not hooks:
-        return True
-
-    output.set_num_hooks(len(hooks))
+        return ret
 
     # Set up the environment like repo would with the forall command.
     try:
@@ -316,11 +373,16 @@ def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, co
     except rh.utils.CalledProcessError as e:
         output.error('Upstream remote/tracking branch lookup',
                      f'{e}\nDid you run repo start?  Is your HEAD detached?')
-        return False
+        return ret._replace(internal_failure=True)
 
     project = rh.Project(name=project_name, dir=proj_dir)
     rel_proj_dir = os.path.relpath(proj_dir, rh.git.find_repo_root())
 
+    # Filter out the hooks to process.
+    hooks = [x for x in hooks if rel_proj_dir not in x.scope]
+    if not hooks:
+        return ret
+
     os.environ.update({
         'REPO_LREV': rh.git.get_commit_for_ref(upstream_branch),
         'REPO_PATH': rel_proj_dir,
@@ -334,50 +396,59 @@ def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, co
             ignore_merged_commits=config.ignore_merged_commits)
     output.set_num_commits(len(commit_list))
 
-    ret = True
-    fixup_list = []
-
-    for commit in commit_list:
-        # Mix in some settings for our hooks.
-        os.environ['PREUPLOAD_COMMIT'] = commit
-        diff = rh.git.get_affected_files(commit)
-        desc = rh.git.get_commit_desc(commit)
-        os.environ['PREUPLOAD_COMMIT_MESSAGE'] = desc
-
-        commit_summary = desc.split('\n', 1)[0]
-        output.commit_start(commit=commit, commit_summary=commit_summary)
-
-        for name, hook, exclusion_scope in hooks:
-            if rel_proj_dir in exclusion_scope:
-                continue
-            output.hook_start(name)
-            hook_results = hook(project, commit, desc, diff)
-            output.hook_finish()
-            (error, warning) = _process_hook_results(hook_results)
-            if error is not None or warning is not None:
-                if warning is not None:
-                    output.hook_warning(warning)
-                if error is not None:
-                    ret = False
-                    output.hook_error(error)
-                for result in hook_results:
-                    if result.fixup_cmd:
-                        fixup_list.append(
-                            (name, commit, result.fixup_cmd, result.files))
-
-    if fixup_list:
-        _attempt_fixes(fixup_list, commit_list, proj_dir)
+    def _run_hook(hook, project, commit, desc, diff):
+        """Run a hook, gather stats, and process its results."""
+        start = datetime.datetime.now()
+        results = hook.hook(project, commit, desc, diff)
+        (error, warning) = _process_hook_results(results)
+        duration = datetime.datetime.now() - start
+        return (hook, results, error, warning, duration)
+
+    with concurrent.futures.ThreadPoolExecutor(max_workers=jobs) as executor:
+        for commit in commit_list:
+            # Mix in some settings for our hooks.
+            os.environ['PREUPLOAD_COMMIT'] = commit
+            diff = rh.git.get_affected_files(commit)
+            desc = rh.git.get_commit_desc(commit)
+            os.environ['PREUPLOAD_COMMIT_MESSAGE'] = desc
+
+            commit_summary = desc.split('\n', 1)[0]
+            output.commit_start(hooks, commit, commit_summary)
+
+            futures = (
+                executor.submit(_run_hook, hook, project, commit, desc, diff)
+                for hook in hooks
+            )
+            future_results = (
+                x.result() for x in concurrent.futures.as_completed(futures)
+            )
+            for hook, hook_results, error, warning, duration in future_results:
+                ret.add_results(hook_results)
+                if error is not None or warning is not None:
+                    if warning is not None:
+                        output.hook_warning(hook, warning)
+                    if error is not None:
+                        output.hook_error(hook, error)
+                        output.hook_fixups(ret, hook_results)
+                output.hook_finish(hook, duration)
 
     return ret
 
 
-def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=None):
+def _run_project_hooks(
+    project_name: str,
+    proj_dir: Optional[str] = None,
+    jobs: Optional[int] = None,
+    from_git: bool = False,
+    commit_list: Optional[List[str]] = None,
+) -> rh.results.ProjectResults:
     """Run the project-specific hooks in |proj_dir|.
 
     Args:
       project_name: The name of project to run hooks for.
       proj_dir: If non-None, this is the directory the project is in.  If None,
           we'll ask repo.
+      jobs: How many hooks to run in parallel.
       from_git: If true, we are called from git directly and repo should not be
           used.
       commit_list: A list of commits to run hooks against.  If None or empty
@@ -385,7 +456,7 @@ def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=
           uploaded.
 
     Returns:
-      True if everything passed, else False.
+      All the results for this project.
     """
     output = Output(project_name)
 
@@ -409,9 +480,9 @@ def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=
     try:
         # Hooks assume they are run from the root of the project.
         os.chdir(proj_dir)
-        return _run_project_hooks_in_cwd(project_name, proj_dir, output,
-                                         from_git=from_git,
-                                         commit_list=commit_list)
+        return _run_project_hooks_in_cwd(
+            project_name, proj_dir, output, jobs=jobs, from_git=from_git,
+            commit_list=commit_list)
     finally:
         output.finish()
         os.chdir(pwd)
@@ -420,6 +491,7 @@ def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=
 def _run_projects_hooks(
     project_list: List[str],
     worktree_list: List[Optional[str]],
+    jobs: Optional[int] = None,
     from_git: bool = False,
     commit_list: Optional[List[str]] = None,
 ) -> bool:
@@ -428,6 +500,7 @@ def _run_projects_hooks(
     Args:
       project_list: List of project names.
       worktree_list: List of project checkouts.
+      jobs: How many hooks to run in parallel.
       from_git: If true, we are called from git directly and repo should not be
           used.
       commit_list: A list of commits to run hooks against.  If None or empty
@@ -437,20 +510,24 @@ def _run_projects_hooks(
     Returns:
       True if everything passed, else False.
     """
-    ret = True
+    results = []
     for project, worktree in zip(project_list, worktree_list):
-        if not _run_project_hooks(
+        result = _run_project_hooks(
             project,
             proj_dir=worktree,
+            jobs=jobs,
             from_git=from_git,
             commit_list=commit_list,
-        ):
-            ret = False
+        )
+        results.append(result)
+        if result:
             # If a repo had failures, add a blank line to help break up the
             # output.  If there were no failures, then the output should be
             # very minimal, so we don't add it then.
             print('', file=sys.stderr)
-    return ret
+
+    _attempt_fixes(results)
+    return not any(results)
 
 
 def main(project_list, worktree_list=None, **_kwargs):
@@ -528,6 +605,11 @@ def direct_main(argv):
                         'hooks get run, since some hooks are project-specific.'
                         'If not specified, `repo` will be used to figure this '
                         'out based on the dir.')
+    parser.add_argument('-j', '--jobs', type=int,
+                        help='Run up to this many hooks in parallel. Setting '
+                        'to 1 forces serial execution, and the default '
+                        'automatically chooses an appropriate number for the '
+                        'current system.')
     parser.add_argument('commits', nargs='*',
                         help='Check specific commits')
     opts = parser.parse_args(argv)
@@ -553,8 +635,8 @@ def direct_main(argv):
             parser.error(f"Couldn't identify the project of {opts.dir}")
 
     try:
-        if _run_projects_hooks([opts.project], [opts.dir], from_git=opts.git,
-                               commit_list=opts.commits):
+        if _run_projects_hooks([opts.project], [opts.dir], jobs=opts.jobs,
+                               from_git=opts.git, commit_list=opts.commits):
             return 0
     except KeyboardInterrupt:
         print('Aborting execution early due to user interrupt', file=sys.stderr)
diff --git a/rh/hooks.py b/rh/hooks.py
index c90c0a3..6cb92a0 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -346,15 +346,17 @@ def check_bpfmt(project, commit, _desc, diff, options=None):
 
     bpfmt = options.tool_path('bpfmt')
     bpfmt_options = options.args((), filtered)
-    cmd = [bpfmt, '-l'] + bpfmt_options
+    cmd = [bpfmt, '-d'] + bpfmt_options
+    fixup_cmd = [bpfmt, '-w']
+    if '-s' in bpfmt_options:
+        fixup_cmd.append('-s')
+    fixup_cmd.append('--')
+
     ret = []
     for d in filtered:
         data = rh.git.get_file_content(commit, d.file)
         result = _run(cmd, input=data)
         if result.stdout:
-            fixup_cmd = [bpfmt, '-w']
-            if '-s' in bpfmt_options:
-                fixup_cmd.append('-s')
             ret.append(rh.results.HookResult(
                 'bpfmt', project, commit,
                 error=result.stdout,
@@ -621,7 +623,7 @@ release notes, you need to include a starting and closing quote.
 Multi-line Relnote example:
 
 Relnote: "Added a new API `Class#getSize` to get the size of the class.
-This is useful if you need to know the size of the class."
+    This is useful if you need to know the size of the class."
 
 Single-line Relnote example:
 
@@ -858,13 +860,14 @@ def check_gofmt(project, commit, _desc, diff, options=None):
         return None
 
     gofmt = options.tool_path('gofmt')
-    cmd = [gofmt, '-l'] + options.args((), filtered)
+    cmd = [gofmt, '-l'] + options.args()
+    fixup_cmd = [gofmt, '-w'] + options.args()
+
     ret = []
     for d in filtered:
         data = rh.git.get_file_content(commit, d.file)
         result = _run(cmd, input=data)
         if result.stdout:
-            fixup_cmd = [gofmt, '-w']
             ret.append(rh.results.HookResult(
                 'gofmt', project, commit, error=result.stdout,
                 files=(d.file,), fixup_cmd=fixup_cmd))
@@ -943,11 +946,9 @@ def check_rustfmt(project, commit, _desc, diff, options=None):
         # TODO(b/164111102): rustfmt stable does not support --check on stdin.
         # If no error is reported, compare stdin with stdout.
         if data != result.stdout:
-            msg = ('To fix, please run: ' +
-                   rh.shell.cmd_to_str(cmd + [d.file]))
             ret.append(rh.results.HookResult(
-                'rustfmt', project, commit, error=msg,
-                files=(d.file,)))
+                'rustfmt', project, commit, error='Files not formatted',
+                files=(d.file,), fixup_cmd=cmd))
     return ret
 
 
@@ -1013,7 +1014,7 @@ def check_android_test_mapping(project, commit, _desc, diff, options=None):
 def check_aidl_format(project, commit, _desc, diff, options=None):
     """Checks that AIDL files are formatted with aidl-format."""
     # All *.aidl files except for those under aidl_api directory.
-    filtered = _filter_diff(diff, [r'\.aidl$'], [r'/aidl_api/'])
+    filtered = _filter_diff(diff, [r'\.aidl$'], [r'(^|/)aidl_api/'])
     if not filtered:
         return None
     aidl_format = options.tool_path('aidl-format')
diff --git a/rh/results.py b/rh/results.py
index a754cef..65e0052 100644
--- a/rh/results.py
+++ b/rh/results.py
@@ -16,7 +16,7 @@
 
 import os
 import sys
-from typing import List, Optional
+from typing import List, NamedTuple, Optional
 
 _path = os.path.realpath(__file__ + '/../..')
 if sys.path[0] != _path:
@@ -50,9 +50,11 @@ class HookResult(object):
         self.fixup_cmd = fixup_cmd
 
     def __bool__(self):
+        """Whether this result is an error."""
         return bool(self.error)
 
     def is_warning(self):
+        """Whether this result is a non-fatal warning."""
         return False
 
 
@@ -67,7 +69,37 @@ class HookCommandResult(HookResult):
         self.result = result
 
     def __bool__(self):
-        return self.result.returncode not in (None, 0)
+        """Whether this result is an error."""
+        return self.result.returncode not in (None, 0, 77)
 
     def is_warning(self):
+        """Whether this result is a non-fatal warning."""
         return self.result.returncode == 77
+
+
+class ProjectResults(NamedTuple):
+    """All results for a single project."""
+
+    project: str
+    workdir: str
+
+    # All the results from running all the hooks.
+    results: List[HookResult] = []
+
+    # Whether there were any non-hook related errors.  For example, trying to
+    # parse the project configuration.
+    internal_failure: bool = False
+
+    def add_results(self, results: Optional[List[HookResult]]) -> None:
+        """Add |results| to our results."""
+        if results:
+            self.results.extend(results)
+
+    @property
+    def fixups(self):
+        """Yield results that have a fixup available."""
+        yield from (x for x in self.results if x and x.fixup_cmd)
+
+    def __bool__(self):
+        """Whether there are any errors in this set of results."""
+        return self.internal_failure or any(self.results)
diff --git a/rh/results_unittest.py b/rh/results_unittest.py
new file mode 100755
index 0000000..93d909e
--- /dev/null
+++ b/rh/results_unittest.py
@@ -0,0 +1,105 @@
+#!/usr/bin/env python3
+# Copyright 2023 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unittests for the results module."""
+
+import os
+import sys
+import unittest
+
+_path = os.path.realpath(__file__ + '/../..')
+if sys.path[0] != _path:
+    sys.path.insert(0, _path)
+del _path
+
+# We have to import our local modules after the sys.path tweak.  We can't use
+# relative imports because this is an executable program, not a module.
+# pylint: disable=wrong-import-position
+import rh
+import rh.results
+import rh.utils
+
+
+COMPLETED_PROCESS_PASS = rh.utils.CompletedProcess(returncode=0)
+COMPLETED_PROCESS_FAIL = rh.utils.CompletedProcess(returncode=1)
+COMPLETED_PROCESS_WARN = rh.utils.CompletedProcess(returncode=77)
+
+
+class HookResultTests(unittest.TestCase):
+    """Verify behavior of HookResult object."""
+
+    def test_error_warning(self):
+        """Check error & warning handling."""
+        # No errors.
+        result = rh.results.HookResult('hook', 'project', 'HEAD', False)
+        self.assertFalse(result)
+        self.assertFalse(result.is_warning())
+
+        # An error.
+        result = rh.results.HookResult('hook', 'project', 'HEAD', True)
+        self.assertTrue(result)
+        self.assertFalse(result.is_warning())
+
+
+class HookCommandResultTests(unittest.TestCase):
+    """Verify behavior of HookCommandResult object."""
+
+    def test_error_warning(self):
+        """Check error & warning handling."""
+        # No errors.
+        result = rh.results.HookCommandResult(
+            'hook', 'project', 'HEAD', COMPLETED_PROCESS_PASS)
+        self.assertFalse(result)
+        self.assertFalse(result.is_warning())
+
+        # An error.
+        result = rh.results.HookCommandResult(
+            'hook', 'project', 'HEAD', COMPLETED_PROCESS_FAIL)
+        self.assertTrue(result)
+        self.assertFalse(result.is_warning())
+
+        # A warning.
+        result = rh.results.HookCommandResult(
+            'hook', 'project', 'HEAD', COMPLETED_PROCESS_WARN)
+        self.assertFalse(result)
+        self.assertTrue(result.is_warning())
+
+
+class ProjectResultsTests(unittest.TestCase):
+    """Verify behavior of ProjectResults object."""
+
+    def test_error_warning(self):
+        """Check error & warning handling."""
+        # No errors.
+        result = rh.results.ProjectResults('project', 'workdir')
+        self.assertFalse(result)
+
+        # Warnings are not errors.
+        result.add_results([
+            rh.results.HookResult('hook', 'project', 'HEAD', False),
+            rh.results.HookCommandResult(
+                'hook', 'project', 'HEAD', COMPLETED_PROCESS_WARN),
+        ])
+        self.assertFalse(result)
+
+        # Errors are errors.
+        result.add_results([
+            rh.results.HookResult('hook', 'project', 'HEAD', True),
+        ])
+        self.assertTrue(result)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/rh/utils.py b/rh/utils.py
index 157e31b..4f1a063 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -369,18 +369,27 @@ def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None,
         old_sigint = signal.getsignal(signal.SIGINT)
         handler = functools.partial(_kill_child_process, proc, int_timeout,
                                     kill_timeout, cmd, old_sigint)
-        signal.signal(signal.SIGINT, handler)
+        # We have to ignore ValueError in case we're run from a thread.
+        try:
+            signal.signal(signal.SIGINT, handler)
+        except ValueError:
+            old_sigint = None
 
         old_sigterm = signal.getsignal(signal.SIGTERM)
         handler = functools.partial(_kill_child_process, proc, int_timeout,
                                     kill_timeout, cmd, old_sigterm)
-        signal.signal(signal.SIGTERM, handler)
+        try:
+            signal.signal(signal.SIGTERM, handler)
+        except ValueError:
+            old_sigterm = None
 
         try:
             (result.stdout, result.stderr) = proc.communicate(input)
         finally:
-            signal.signal(signal.SIGINT, old_sigint)
-            signal.signal(signal.SIGTERM, old_sigterm)
+            if old_sigint is not None:
+                signal.signal(signal.SIGINT, old_sigint)
+            if old_sigterm is not None:
+                signal.signal(signal.SIGTERM, old_sigterm)
 
             if popen_stdout:
                 # The linter is confused by how stdout is a file & an int.