health-check: Add infra checks

Adds presubmit checks for both lucicfg files and python recipes.

Both can "fix" issues by re-generating configs / test expectations.

This adds depot_tools as a submodule to crosvm, which contains the
required tools for luci.

Does not validate the infra config. Unfortunately this is only
possible with luci authentication which we do not have inside
Docker.

BUG=b:242605601
TEST=./tools/health-check with changes to config and recipes.

Change-Id: I1bf18ebac698e44df7d6a0d6c8e9c26bcfde364b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3832783
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Dennis Kempin <denniskempin@google.com>
This commit is contained in:
Dennis Kempin 2022-08-16 17:58:38 +00:00 committed by crosvm LUCI
parent 13cedb8b0b
commit 38b9268cb8
5 changed files with 53 additions and 3 deletions

3
.gitmodules vendored
View file

@ -13,3 +13,6 @@
[submodule "third_party/vmm_vhost"]
path = third_party/vmm_vhost
url = https://chromium.googlesource.com/chromiumos/third_party/rust-vmm/vhost/
[submodule "third_party/depot_tools"]
path = third_party/depot_tools
url = https://chromium.googlesource.com/chromium/tools/depot_tools.git

1
third_party/depot_tools vendored Submodule

@ -0,0 +1 @@
Subproject commit 268d645853ee8e1b884260049e5464a5ca2d8a30

View file

@ -127,6 +127,25 @@ def check_copyright_header(context: CheckContext):
raise Exception(f"Bad copyright header: {file}")
def check_infra_configs(context: CheckContext):
"Validate luci configs by sending them to luci-config."
lucicfg = cmd("third_party/depot_tools/lucicfg")
for file in context.modified_files:
if context.fix:
lucicfg("fmt", file).fg()
lucicfg("generate", file).fg()
lucicfg("fmt --dry-run", file).fg(quiet=True)
# TODO: Validate config files. Requires authentication with luci inside docker.
def check_infra_tests(context: CheckContext):
"Run recipe.py tests, all of them, regardless of which files were modified."
recipes = cmd("infra/recipes.py").add_path("third_party/depot_tools")
if context.fix:
recipes("test train --py3-only").fg()
recipes("test run --py3-only").fg(quiet=True)
# List of all checks and on which files they should run.
CHECKS: List[Check] = [
Check(
@ -137,6 +156,7 @@ CHECKS: List[Check] = [
Check(
check_rust_format,
files=["**.rs"],
can_fix=True,
),
Check(
check_rust_lockfiles,
@ -161,6 +181,17 @@ CHECKS: List[Check] = [
files=["**.py"],
python_tools=True,
exclude=["infra/recipes.py"],
can_fix=True,
),
Check(
check_infra_configs,
files=["infra/config/**.star"],
can_fix=True,
),
Check(
check_infra_tests,
files=["infra/**.py"],
can_fix=True,
),
Check(
check_markdown_format,
@ -169,6 +200,7 @@ CHECKS: List[Check] = [
"infra/README.recipes.md",
"docs/book/src/appendix/memory_layout.md",
],
can_fix=True,
),
Check(check_crlf_line_endings),
]

View file

@ -315,6 +315,13 @@ class Command(object):
cmd.env_vars = {**self.env_vars, key: value}
return cmd
def add_path(self, new_path: str):
path_var = self.env_vars.get("PATH", os.environ.get("PATH", ""))
cmd = Command()
cmd.args = self.args
cmd.env_vars = {**self.env_vars, "PATH": f"{path_var}:{new_path}"}
return cmd
def foreach(self, arguments: Iterable[Any], batch_size: int = 1):
"""
Yields a new command for each entry in `arguments`.

View file

@ -48,6 +48,9 @@ class Check(NamedTuple):
# List of globs to exclude from this check
exclude: List[str] = []
# Whether or not this check can fix issues.
can_fix: bool = False
@property
def name(self):
name = self.check_function.__name__
@ -142,7 +145,7 @@ def run_checks(
else:
modified_files = [f for (s, f) in file_diff if s in ("M", "A")]
success = True
failed_checks: List[Check] = []
for check in checks_list:
context = CheckContext(
fix=fix,
@ -153,5 +156,9 @@ def run_checks(
)
if context.modified_files:
if not run_check(check, context):
success = False
return success
failed_checks.append(check)
if any(c.can_fix for c in failed_checks):
print("")
print("Some of the issues above can be fixed automatically with:")
print("./tools/health-check --fix")
return len(failed_checks) == 0