ec: Replace presubmit_check.sh

Replace presubmit_check.sh and zephyr_check_testcase_yaml.py with
python scripts which handle PRESUBMIT_COMMIT, PRESUBMIT_FILES, and also
work outside of the chroot.

BUG=None
TEST=on various paths

Change-Id: If7ff5e5d238a5846c19894bb288e96052f99aae2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4497260
Commit-Queue: Jack Rosenthal <jrosenth@chromium.org>
Tested-by: Jeremy Bettis <jbettis@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Jeremy Bettis <jbettis@chromium.org>
Auto-Submit: Jeremy Bettis <jbettis@chromium.org>

6 files changed, 166 insertions, 122 deletions

diff --git a/util/check_cprints.py b/util/check_cprints.py
new file mode 100755
index 0000000000..38de40c5c7
--- /dev/null
+++ b/util/check_cprints.py
@@ -0,0 +1,39 @@
+#!/usr/bin/env vpython3
+# Copyright 2023 The ChromiumOS Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Checks files for invalid CPRINTS calls."""
+
+import re
+import sys
+from typing import List, Optional
+
+import preupload.lib
+
+
+CPRINTS_RE = re.compile(r'(CPRINTS|cprints)[^"]*"[^"]*\\n"')
+
+
+def main(argv: Optional[List[str]] = None) -> Optional[int]:
+    """Look at all files passed in on commandline for invalid CPRINTS calls."""
+    return_code = 0
+    parser = preupload.lib.argument_parser()
+
+    args = parser.parse_args(argv)
+    for filename in args.filename:
+        lines = preupload.lib.cat_file(args, filename).splitlines()
+        for linenum, line in enumerate(lines, start=1):
+            if CPRINTS_RE.search(line):
+                print(
+                    "error: CPRINTS strings should not include newline "
+                    f"characters\n{filename}:{linenum}: {line}",
+                    file=sys.stderr,
+                )
+                return_code = 1
+
+    return return_code
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))
diff --git a/util/check_ztest.py b/util/check_ztest.py
new file mode 100755
index 0000000000..69cb6a08df
--- /dev/null
+++ b/util/check_ztest.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env vpython3
+# Copyright 2023 The ChromiumOS Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Checks files for invalid ZTEST decls."""
+
+import re
+import sys
+from typing import List, Optional
+
+import preupload.lib
+
+
+ZTEST_RE = re.compile(
+    r"(ZTEST|ZTEST_F|ZTEST_USER|ZTEST_USER_F)\(\w+,\s*($|\w+)"
+)
+
+
+def main(argv: Optional[List[str]] = None) -> Optional[int]:
+    """Look at all files passed in on commandline for ZTEST decls where the
+    test is not named test_.
+    """
+    return_code = 0
+    parser = preupload.lib.argument_parser()
+
+    args = parser.parse_args(argv)
+    for filename in args.filename:
+        lines = preupload.lib.cat_file(args, filename).splitlines()
+        line_iter = iter(enumerate(lines, start=1))
+        for linenum, line in line_iter:
+            line = line.rstrip("\n")
+            match = ZTEST_RE.search(line)
+            if match and match.group(2) == "":
+                linenum, line2 = next(line_iter)
+                line += line2.rstrip("\n")
+                match = ZTEST_RE.search(line)
+            if match and not match.group(2).startswith("test_"):
+                print(
+                    "error: 'test_' prefix missing from test function name"
+                    f"\n{filename}:{linenum}: {match.group(2)}",
+                    file=sys.stderr,
+                )
+                return_code = 1
+
+    return return_code
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))
diff --git a/util/presubmit_check.sh b/util/presubmit_check.sh
deleted file mode 100755
index 08b548b631..0000000000
--- a/util/presubmit_check.sh
+++ /dev/null
@@ -1,36 +0,0 @@
-#!/bin/bash
-#
-# Copyright 2014 The ChromiumOS Authors
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-# Verify there is no CPRINTS("....\n", ...) statements added to the code.
-upstream_branch="$(git rev-parse --abbrev-ref --symbolic-full-name @\{u\} \
-    2>/dev/null)"
-if [[ -z ${upstream_branch} ]]; then
-  echo "Current branch does not have an upstream branch" >&2
-  exit 1
-fi
-# This will print the offending CPRINTS invocations, if any, and the names of
-# the files they are in.
-if git diff --no-ext-diff "${upstream_branch}" HEAD |
-    grep -e '^+\(.*CPRINTS(.*\\n"\|++\)' |
-    grep CPRINTS -B1 >&2 ; then
-  echo "error: CPRINTS strings should not include newline characters" >&2
-  exit 1
-fi
-
-# Check for missing 'test_' prefix from ZTEST definitions
-if git diff --no-ext-diff "${upstream_branch}" HEAD |
-     pcregrep -M "^\+(ZTEST|ZTEST_F|ZTEST_USER|ZTEST_USER_F)\(\w+,[\n\+|\s]*\w+\)" |
-     pcregrep -vM "\(\w+,[\n\+]*\s*test_\w+\)"; then
-  echo "error: 'test_' prefix missing from test function name" >&2
-  exit 1
-fi
-
-# Validate testcase.yaml files
-if ! git diff --no-ext-diff --name-only "${upstream_branch}" HEAD |
-     util/zephyr_check_testcase_yaml.py - ; then
-  echo "Errors detected while validating testcase.yaml files"
-  exit 1
-fi
diff --git a/util/preupload/__init__.py b/util/preupload/__init__.py
new file mode 100644
index 0000000000..96bb372274
--- /dev/null
+++ b/util/preupload/__init__.py
@@ -0,0 +1,3 @@
+# Copyright 2023 The ChromiumOS Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
diff --git a/util/preupload/lib.py b/util/preupload/lib.py
new file mode 100644
index 0000000000..87a1226507
--- /dev/null
+++ b/util/preupload/lib.py
@@ -0,0 +1,34 @@
+# Lint as: python3
+# Copyright 2023 The ChromiumOS Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Common utilities for pre-upload checks."""
+
+import argparse
+from pathlib import Path
+import subprocess
+
+
+PRE_SUBMIT = "pre-submit"
+
+
+def cat_file(args, filename) -> str:
+    """Read a file either from disk, or from a git commit."""
+    if args.commit == PRE_SUBMIT:
+        with open(filename, encoding="utf-8") as infile:
+            return infile.read()
+    return subprocess.run(
+        ["git", "show", f"{args.commit}:{filename}"],
+        universal_newlines=True,
+        stdout=subprocess.PIPE,
+        check=True,
+    ).stdout
+
+
+def argument_parser():
+    """Returns an ArgumentParser with standard options configured."""
+    parser = argparse.ArgumentParser()
+    parser.add_argument("-c", "--commit", default=PRE_SUBMIT)
+    parser.add_argument("filename", nargs="+", type=Path)
+    return parser
diff --git a/util/zephyr_check_testcase_yaml.py b/util/zephyr_check_testcase_yaml.py
index 82a8320a72..51ff71db89 100755
--- a/util/zephyr_check_testcase_yaml.py
+++ b/util/zephyr_check_testcase_yaml.py
@@ -13,99 +13,53 @@
 # >
 # [VPYTHON:END]
 
-import argparse
-import glob
-from pathlib import Path
 import sys
-from typing import List
+from typing import List, Optional
 
-from twister_launcher import EC_TEST_PATHS
+import preupload.lib
 import yaml  # pylint: disable=import-error
 
 
-def check_extra_args(filepath: Path):
-    """Check if extra_args references blocked fields"""
-    errors = []
-
-    blocked_fields = {
-        "CONF_FILE": "extra_conf_files",
-        "OVERLAY_CONFIG": "extra_overlay_confs",
-        "DTC_OVERLAY_FILE": "extra_dtc_overlay_files",
-    }
-
-    with open(filepath, "r") as file:
-        data = yaml.load(file, Loader=yaml.SafeLoader)
-
-    def scan_section(section):
-        if "extra_args" in section:
-            for field in blocked_fields:
-                if field in section["extra_args"]:
-                    errors.append(
-                        f" * Don't specify {field} in `extra_args`. "
-                        f"Use `{blocked_fields[field]}` ({filepath})"
-                    )
-
-    if "common" in data:
-        scan_section(data["common"])
-    if "tests" in data:
-        for section in data["tests"]:
-            scan_section(data["tests"][section])
-
-    return errors
-
-
-def validate_files(files: List[Path]) -> List[str]:
-    """Run checks on a list of file paths."""
-    errors = []
-
-    checkers = [check_extra_args]
-
-    for file in files:
-        if not file.name == "testcase.yaml":
+BLOCKED_FIELDS = {
+    "CONF_FILE": "extra_conf_files",
+    "OVERLAY_CONFIG": "extra_overlay_confs",
+    "DTC_OVERLAY_FILE": "extra_dtc_overlay_files",
+}
+
+
+def main(argv: Optional[List[str]] = None) -> Optional[int]:
+    """Look at all testcase.yaml files passed in on commandline for invalid fields."""
+    return_code = 0
+    parser = preupload.lib.argument_parser()
+
+    args = parser.parse_args(argv)
+    for filename in args.filename:
+        if filename.name == "/testcase.yaml":
+            lines = preupload.lib.cat_file(args, filename)
+            data = yaml.load(lines, Loader=yaml.SafeLoader)
+
+            def scan_section(section, filename):
+                nonlocal return_code
+                if "extra_args" in section:
+                    for field, replacement in BLOCKED_FIELDS.items():
+                        if field in section["extra_args"]:
+                            print(
+                                f"error: Don't specify {field} in "
+                                f"`extra_args`. Use `{replacement}`: "
+                                f"{filename}",
+                                file=sys.stderr,
+                            )
+                            return_code = 1
+
+            if "common" in data:
+                scan_section(data["common"], filename)
+            if "tests" in data:
+                for section in data["tests"]:
+                    scan_section(data["tests"][section], filename)
             continue
-        for check in checkers:
-            errors.extend(check(file))
 
-    return errors
+    return return_code
 
 
 if __name__ == "__main__":
-    parser = argparse.ArgumentParser()
-    parser.add_argument(
-        "files",
-        nargs="*",
-        type=Path,
-        help="List of files to validate. If blank, scan entire EC repo. "
-        "If '-', read a newline-separated list of files from stdin",
-    )
-
-    args = parser.parse_args()
-
-    if len(args.files) == 0:
-        ec_dir = Path(__file__).resolve().parent.parent
-        file_list = []
-
-        for p in EC_TEST_PATHS:
-            file_list.extend(
-                [
-                    Path(f)
-                    for f in glob.glob(
-                        str(ec_dir / p / "**/testcase.yaml"),
-                    )
-                ]
-            )
-
-    elif args.files[0] == Path("-"):
-        # Read from stdin
-        file_list = [Path(line.strip()) for line in sys.stdin.readlines()]
-        file_list.extend(args.files[1:])
-    else:
-        file_list = args.files
-
-    all_errors = validate_files(file_list)
-
-    if all_errors:
-        for error in all_errors:
-            sys.stderr.write(error)
-            sys.stderr.write("\n")
-        sys.exit(1)
+    sys.exit(main(sys.argv[1:]))