results: filter out warnings from the "is an error" check

Stop treating warnings as errors, and fix the reporting to run if
the result is a warning or an error.

Bug: 294133100
Test: unittests

Change-Id: Ic238b910a215ae5ec7c7c590299b2403b77c99d5

4 files changed, 108 insertions, 2 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 59b601a..1f71868 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -222,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'
diff --git a/rh/results.py b/rh/results.py
index 8adcf6b..65e0052 100644
--- a/rh/results.py
+++ b/rh/results.py
@@ -70,7 +70,7 @@ class HookCommandResult(HookResult):
 
     def __bool__(self):
         """Whether this result is an error."""
-        return self.result.returncode not in (None, 0)
+        return self.result.returncode not in (None, 0, 77)
 
     def is_warning(self):
         """Whether this result is a non-fatal warning."""
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()