Skip to content

Commit

Permalink
Add tests for hook implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
bwt-sloanj committed Sep 25, 2024
1 parent e98edf2 commit 4ce4bf1
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 20 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,5 @@ cython_debug/
# and can be added to the global gitignore or merged into this file. For a more nuclear
# option (not recommended) you can uncomment the following to ignore the entire idea folder.
#.idea/

.vscode
41 changes: 30 additions & 11 deletions checkpatch_hook/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

logging.basicConfig()
logger = getLogger(__name__)
logger.setLevel(logging.INFO)

checkpatch_script_path = importlib.resources.files("checkpatch_hook.scripts").joinpath(
"checkpatch.pl"
Expand Down Expand Up @@ -55,6 +54,7 @@ def _parse_args() -> dict:
class ConfigFile:
DEFAULT_DIR_KEY = "__default__"
mandatory_keys = ["DIR_CONFIGS"]
# TODO: with YAML anchors there wouldn't really be a need for this approach
magic_error_keys = {"errors_enabled": "ERRORS_COMMON", "errors_ignored": "IGNORES_COMMON"}
optional_keys = ["IGNORED_FILES"]
# TODO: Use schema based validation e.g., cfgv
Expand Down Expand Up @@ -114,6 +114,7 @@ def pre_commit_hook(config_file: Path, commit_files: list[Path], fix_inplace: bo
config = config_file_obj.load_config()
logger.debug(f"Config file @ {config_file.resolve()}")

result = True
errors: DefaultDict[str, list] = defaultdict(list)
# accumulate checkpatch errors for each configured dir
for config_dir, dconfig in config["DIR_CONFIGS"].items():
Expand All @@ -126,24 +127,39 @@ def pre_commit_hook(config_file: Path, commit_files: list[Path], fix_inplace: bo
patch_files = [
p
for p in commit_files
if p.is_relative_to(config_dir)
if check_config_applicable(config, config_dir, p)
and p not in config.get("IGNORED_FILES", [])
and not p.is_symlink()
]
if patch_files:
_run_checkpatch(patch_files, fix_inplace, errors, config_dir, **post_dconfig)
result_dir = _run_checkpatch(
patch_files, fix_inplace, errors, config_dir, **post_dconfig
)
if not result_dir:
result = result_dir
else:
logger.debug(f"No files to check in {config_dir}")

# print errors
for filename, f_errors in errors.items():
logger.error(f"{filename}:")
for error in f_errors:
logger.error(f' {error["line"]}: {error["message"]}')
if errors:
# log errors
for filename, file_errors in errors.items():
for error in file_errors:
logger.error("%s:%s: %s", filename, error["line"], error["message"])
if not result:
sys.exit(1)


def check_config_applicable(config: dict, config_dir: Path, path: Path) -> bool:
if str(config_dir) != ".":
return path.is_relative_to(config_dir)
for other_config_dir in config["DIR_CONFIGS"].keys():
if other_config_dir == ConfigFile.DEFAULT_DIR_KEY:
continue
other_config_path = Path(other_config_dir)
if path.is_relative_to(other_config_path):
return False
return True


def _run_checkpatch(
patch_files: list[Path],
fix_inplace: bool,
Expand All @@ -152,7 +168,7 @@ def _run_checkpatch(
errors_enabled: list,
errors_ignored: list,
max_line_length: int | str | None,
) -> None:
) -> bool:
cmd = [
str(checkpatch_script_path),
"--strict", # Be more annoying
Expand Down Expand Up @@ -183,6 +199,8 @@ def _run_checkpatch(

if result.returncode != 0:
_process_checkpatch_errors(result.stdout.decode(), errors)
return False
return True


def _process_checkpatch_errors(output: str, errors: DefaultDict[str, list]) -> None:
Expand All @@ -203,9 +221,10 @@ def main() -> None:
Entry point to be used when run as hook script.
"""
args = _parse_args()
logger.debug(f"Running with args: {args}")
if args["verbose"]:
logger.setLevel(logging.DEBUG)
else:
logger.setLevel(logging.INFO)
pre_commit_hook(args["config_file"], args["files"], args["fix_inplace"])


Expand Down
8 changes: 0 additions & 8 deletions checkpatch_hook/data/checkpatch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,5 @@ DIR_CONFIGS:
__default__:
errors_ignored:
- IGNORES_COMMON
- VOLATILE
- NEW_TYPEDEFS
- AVOID_EXTERNS
- MACRO_ARG_REUSE
- PREFER_KERNEL_TYPES
- PREFER_PACKED
- SSCANF_TO_KSTRTO
max_line_length: 120

IGNORED_FILES: []
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dev-dependencies = [
"pytest>=8.3.2",
"coverage>=7.6.1",
"pytest-cov>=5.0.0",
"pytest-subprocess>=1.5.2",
]

[tool.pytest.ini_options]
Expand Down
150 changes: 150 additions & 0 deletions tests/test_hook.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import logging
from pathlib import Path
from textwrap import dedent

from pytest import LogCaptureFixture, MonkeyPatch, raises
from pytest_subprocess import FakeProcess

from checkpatch_hook import logger, main


def _register_pass(fp: FakeProcess) -> None:
fp.register([fp.any()], returncode=0)


def _register_fail(fp: FakeProcess, stdout: str | None = None) -> None:
fp.register([fp.any()], returncode=1, stdout=stdout)


def _invoke_hook(monkeypatch: MonkeyPatch, args: list[str]) -> None:
monkeypatch.setattr("sys.argv", ["checkpatch"] + args)
main()


def test_default_config_is_applied_pass(fp: FakeProcess, monkeypatch: MonkeyPatch) -> None:
args = ["--verbose", "test1.c"]
common_ignores = [
"BAD_SIGN_OFF",
"SPDX_LICENSE_TAG",
"FILE_PATH_CHANGES",
"NOT_UNIFIED_DIFF",
"LINUX_VERSION_CODE",
"CONSTANT_COMPARISON",
"OPEN_ENDED_LINE",
"UNNECESSARY_PARENTHESES",
"GERRIT_CHANGE_ID",
"COMMIT_LOG_LONG_LINE",
"EMAIL_SUBJECT",
"GIT_COMMIT_ID",
]

_register_pass(fp)

_invoke_hook(monkeypatch, args)

invocation = " ".join(fp.calls[0])

for ignore in common_ignores:
assert ignore in invocation

assert "--types" not in invocation
assert "--fix-inplace" not in invocation
assert "--max-line-length" not in invocation
assert "--file test1.c" in invocation


def test_default_config_fix_inplace_fail(fp: FakeProcess, monkeypatch: MonkeyPatch) -> None:
args = ["--fix-inplace", "test1.c"]

_register_fail(fp)

with raises(SystemExit):
_invoke_hook(monkeypatch, args)

invocation = " ".join(fp.calls[0])

assert "--fix-inplace" in invocation
assert "--file test1.c" in invocation


def test_default_config_no_files_pass(fp: FakeProcess, monkeypatch: MonkeyPatch) -> None:
_register_pass(fp)

_invoke_hook(monkeypatch, [])

assert len(fp.calls) == 0


def test_specified_config_is_applied_pass(
fp: FakeProcess, monkeypatch: MonkeyPatch, tmp_path: Path
) -> None:
config_path = tmp_path / "test_config.yaml"
config_text = dedent(
"""\
DIR_CONFIGS:
__default__:
errors_ignored:
- UNNECESSARY_PARENTHESES
max_line_length: '120'
dir2:
errors_enabled:
- GERRIT_CHANGE_ID
max_line_length: '100'
"""
)
config_path.write_text(config_text)
args = ["--config-file", str(config_path), "test1.c", "dir2/test2.h"]

_register_pass(fp)
_register_pass(fp)

_invoke_hook(monkeypatch, args)

assert len(fp.calls) == 2

invocation_default = " ".join(fp.calls[0])
invocation_dir2 = " ".join(fp.calls[1])

assert "--ignore UNNECESSARY_PARENTHESES" in invocation_default
assert "--types" not in invocation_default
assert "--fix-inplace" not in invocation_default
assert "--max-line-length=120" in invocation_default
assert "--file test1.c" in invocation_default
assert "test2.h" not in invocation_default

assert "--ignore" not in invocation_dir2
assert "--types GERRIT_CHANGE_ID" in invocation_dir2
assert "--fix-inplace" not in invocation_dir2
assert "--max-line-length=100" in invocation_dir2
assert "--file dir2/test2.h" in invocation_dir2
assert "test1.c" not in invocation_dir2


def test_errors_returned(
fp: FakeProcess, monkeypatch: MonkeyPatch, tmp_path: Path, caplog: LogCaptureFixture
) -> None:
config_path = tmp_path / "test_config.yaml"
config_text = dedent(
"""\
DIR_CONFIGS:
__default__:
errors_ignored:
- UNNECESSARY_PARENTHESES
max_line_length: '120'
dir2:
errors_enabled:
- GERRIT_CHANGE_ID
max_line_length: '100'
"""
)
config_path.write_text(config_text)
args = ["--config-file", str(config_path), "test1.c", "dir2/test2.h"]

_register_fail(fp, "test1.c:101: checkpatch: WARNING: struct should normally be const")
_register_pass(fp)

with caplog.at_level(logging.ERROR, logger.name):
with raises(SystemExit):
_invoke_hook(monkeypatch, args)

assert "WARNING: struct should normally be const" in caplog.records[0].message
16 changes: 15 additions & 1 deletion uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4ce4bf1

Please sign in to comment.