clang-format: handle exit(0) w/no output from git-clang-format am: e0447aa176

Original change: https://android-review.googlesource.com/c/platform/tools/repohooks/+/2096917

Change-Id: If1d90bc868cc8e1ddcfb83e74e1f0f2b09d67805
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 99 insertions, 2 deletions

diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 81f505d..631915d 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -5,6 +5,7 @@ hooks_unittest  = ./rh/hooks_unittest.py
 shell_unittest  = ./rh/shell_unittest.py
 utils_unittest  = ./rh/utils_unittest.py
 android_test_mapping_format_unittest = ./tools/android_test_mapping_format_unittest.py
+clang-format unittest = ./tools/clang-format_unittest.py
 config_test = ./rh/config_test.py --check-env --commit-id ${PREUPLOAD_COMMIT} --commit-msg ${PREUPLOAD_COMMIT_MESSAGE} --repo-root ${REPO_ROOT} -- ${PREUPLOAD_FILES}
 
 [Builtin Hooks]
diff --git a/tools/clang-format.py b/tools/clang-format.py
index ebb6abe..24ef711 100755
--- a/tools/clang-format.py
+++ b/tools/clang-format.py
@@ -82,8 +82,10 @@ def main(argv):
     result = rh.utils.run(cmd, capture_output=True, check=False)
     # Newer versions of git-clang-format will exit 1 when it worked.  Assume a
     # real failure is any exit code above 1, or any time stderr is used, or if
-    # it exited 0/1 but didn't produce anything useful to stdout.
-    if result.returncode > 1 or result.stderr or not result.stdout:
+    # it exited 1 but didn't produce anything useful to stdout.  If it exited 0,
+    # then assume all is well and we'll attempt to parse its output below.
+    if (result.returncode > 1 or result.stderr or
+        (not result.stdout and result.returncode)):
         print(f'clang-format failed:\ncmd: {result.cmdstr}\n'
               f'stdout:\n{result.stdout}\nstderr:\n{result.stderr}',
               file=sys.stderr)
diff --git a/tools/clang-format_unittest.py b/tools/clang-format_unittest.py
new file mode 100755
index 0000000..1128f65
--- /dev/null
+++ b/tools/clang-format_unittest.py
@@ -0,0 +1,94 @@
+#!/usr/bin/env python3
+# Copyright 2022 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 clang-format."""
+
+import contextlib
+from pathlib import Path
+import sys
+import tempfile
+import unittest
+
+
+DIR = Path(__file__).resolve().parent
+sys.path.insert(0, str(DIR.parent))
+
+# 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.utils
+
+
+CLANG_FORMAT = DIR / 'clang-format.py'
+
+
+@contextlib.contextmanager
+def git_clang_format(data: str):
+    """Create a fake git-clang-format script."""
+    with tempfile.TemporaryDirectory(prefix='repohooks-tests') as tempdir:
+        tempdir = Path(tempdir)
+        script = tempdir / 'git-clang-format-fake.sh'
+        script.write_text(f'#!/bin/sh\n{data}', encoding='utf-8')
+        script.chmod(0o755)
+        yield script
+
+
+def run_clang_format(script, args, **kwargs):
+    """Helper to run clang-format.py with fake git-clang-format script."""
+    kwargs.setdefault('capture_output', True)
+    return rh.utils.run(
+        [CLANG_FORMAT, '--git-clang-format', script] + args, **kwargs)
+
+
+class GitClangFormatExit(unittest.TestCase):
+    """Test git-clang-format parsing."""
+
+    def test_diff_exit_0_no_output(self):
+        """Test exit 0 w/no output."""
+        with git_clang_format('exit 0') as script:
+            result = run_clang_format(script, ['--working-tree'])
+            self.assertEqual(result.stdout, '')
+
+    def test_diff_exit_0_stderr(self):
+        """Test exit 0 w/stderr output."""
+        with git_clang_format('echo bad >&2; exit 0') as script:
+            with self.assertRaises(rh.utils.CalledProcessError) as e:
+                run_clang_format(script, ['--working-tree'])
+            self.assertIn('clang-format failed', e.exception.stderr)
+
+    def test_diff_exit_1_no_output(self):
+        """Test exit 1 w/no output."""
+        with git_clang_format('exit 1') as script:
+            with self.assertRaises(rh.utils.CalledProcessError) as e:
+                run_clang_format(script, ['--working-tree'])
+            self.assertIn('clang-format failed', e.exception.stderr)
+
+    def test_diff_exit_1_stderr(self):
+        """Test exit 1 w/stderr."""
+        with git_clang_format('echo bad >&2; exit 1') as script:
+            with self.assertRaises(rh.utils.CalledProcessError) as e:
+                run_clang_format(script, ['--working-tree'])
+            self.assertIn('clang-format failed', e.exception.stderr)
+
+    def test_diff_exit_2(self):
+        """Test exit 2."""
+        with git_clang_format('exit 2') as script:
+            with self.assertRaises(rh.utils.CalledProcessError) as e:
+                run_clang_format(script, ['--working-tree'])
+            self.assertIn('clang-format failed', e.exception.stderr)
+
+
+if __name__ == '__main__':
+    unittest.main()