Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 99 additions & 4 deletions cr_checker/tests/test_cr_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@
# unit tests for the shebang handling in the cr_checker module
from __future__ import annotations

import logging
import importlib.util
import pytest
from datetime import datetime
from pathlib import Path


LOGGER = logging.getLogger(__name__)


# load the cr_checker module
def load_cr_checker_module():
module_path = Path(__file__).resolve().parents[1] / "tool" / "cr_checker.py"
Expand Down Expand Up @@ -170,7 +174,7 @@ def test_process_files_skips_exclusion_with_missing_header(prepare_test_no_heade
files=[test_file],
templates={extension: header_template},
fix=False,
exclusion=[str(test_file)],
exclusion={str(test_file)},
use_mmap=False,
encoding="utf-8",
)
Expand Down Expand Up @@ -431,7 +435,7 @@ def test_exclusion_file_respected_at_root(tmp_path):

exclusion, valid = cr_checker.load_exclusion(exclusion_file)
assert valid is True
assert exclusion == [str(test_file)]
assert exclusion == {str(test_file)}

results = cr_checker.process_files(
files=[test_file],
Expand Down Expand Up @@ -465,7 +469,7 @@ def test_exclusion_file_respected_in_nested_directory(tmp_path):

exclusion, valid = cr_checker.load_exclusion(exclusion_file)
assert valid is True
assert exclusion == [str(test_file)]
assert exclusion == {str(test_file)}

results = cr_checker.process_files(
files=[test_file],
Expand All @@ -481,6 +485,97 @@ def test_exclusion_file_respected_in_nested_directory(tmp_path):
assert test_file.read_text(encoding="utf-8") == original_content


# test that a glob entry in the exclusion file is expanded recursively:
# `.claude/**/*` covers direct children as well as arbitrarily nested files,
# and a literal entry in the same file keeps working alongside it
def test_exclusion_file_expands_glob_pattern(tmp_path, monkeypatch):
cr_checker = load_cr_checker_module()
header_template = load_template("py")

workspace_dir = tmp_path / "workspace"
nested_dir = workspace_dir / ".claude" / "skills" / "some_skill" / "scripts"
nested_dir.mkdir(parents=True)
(workspace_dir / ".claude" / "agents").mkdir()
(workspace_dir / "tool").mkdir()

original_content = "some content\n"
nested_file = nested_dir / "fix_titles.py"
direct_child = workspace_dir / ".claude" / "settings.py"
agent_file = workspace_dir / ".claude" / "agents" / "reviewer.py"
literal_file = workspace_dir / "tool" / "generated.py"
outside_file = workspace_dir / "tool" / "checked.py"
for excluded in (nested_file, direct_child, agent_file, literal_file, outside_file):
excluded.write_text(original_content, encoding="utf-8")

exclusion_file = workspace_dir / "exclusion.txt"
exclusion_file.write_text(
"# AI - Stuff\n.claude/**/*\n\ntool/generated.py\n",
encoding="utf-8",
)

execroot = tmp_path / "execroot"
execroot.mkdir()
monkeypatch.chdir(execroot)
monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace_dir))

exclusion, valid = cr_checker.load_exclusion(exclusion_file)

assert valid is True
excluded_files = sorted(item for item in exclusion if Path(item).is_file())
assert excluded_files == sorted(
str(path) for path in (nested_file, direct_child, agent_file, literal_file)
)
assert str(outside_file) not in exclusion

results = cr_checker.process_files(
files=[nested_file, direct_child, agent_file, literal_file, outside_file],
templates={"py": header_template},
fix=True,
exclusion=exclusion,
use_mmap=False,
encoding="utf-8",
)

assert results["no_copyright"] == 1
assert results["fixed"] == 1
for skipped in (nested_file, direct_child, agent_file, literal_file):
assert skipped.read_text(encoding="utf-8") == original_content
skipped_header_inserted = outside_file.read_text(encoding="utf-8")
assert skipped_header_inserted.startswith(
header_template.format(year=datetime.now().year)
)


# test the pathway where a glob pattern does not match anything
def test_exclusion_file_glob_without_match_is_invalid(tmp_path, monkeypatch, caplog):
cr_checker = load_cr_checker_module()

workspace_dir = tmp_path / "workspace"
nested_dir = workspace_dir / ".claude" / "skills" / "some_skill" / "scripts"
nested_dir.mkdir(parents=True)

original_content = "some content\n"
nested_file = nested_dir / "fix_titles.py"
nested_file.write_text(original_content, encoding="utf-8")

exclusion_file = workspace_dir / "exclusion.txt"
exclusion_file.write_text(
"# This doesn't exists \n.not_here/**/*\n\n.claude/**/*\n",
encoding="utf-8",
)

execroot = tmp_path / "execroot"
execroot.mkdir()
monkeypatch.chdir(execroot)
monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace_dir))

with caplog.at_level(logging.WARNING):
_, valid = cr_checker.load_exclusion(exclusion_file)
assert "Exclusion pattern .not_here/**/* matched nothing." in caplog.text

assert valid is False


# test that a workspace-relative exclusion entry (as produced by e.g. `git ls-files`)
# is still resolved correctly when the process's cwd is not the workspace root, which
# is what happens under `bazel run`/`bazel test` (BUILD_WORKSPACE_DIRECTORY is set to
Expand Down Expand Up @@ -508,7 +603,7 @@ def test_exclusion_file_respected_under_bazel_run_cwd(tmp_path, monkeypatch):

exclusion, valid = cr_checker.load_exclusion(exclusion_file)
assert valid is True
assert exclusion == [str(test_file)]
assert exclusion == {str(test_file)}

collected_files = cr_checker.collect_inputs([relative_entry], exts=["py"])
assert collected_files == [test_file]
Expand Down
56 changes: 39 additions & 17 deletions cr_checker/tool/cr_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

BORDER_FILL_PATTERN = re.compile(r"([/*#'\-=+])\1{4,}")
FILL_CHARS_REGEX = r"[/*#'\-=+]+"
GLOB_CHARS = ("*", "?", "[")

LOGGER = logging.getLogger()

Expand Down Expand Up @@ -165,7 +166,7 @@ def add_template_for_extensions(
return templates


def load_exclusion(path):
def load_exclusion(path: Path) -> tuple[set[str], bool]:
"""
Loads the list of files being excluded from the copyright check.

Expand All @@ -176,23 +177,45 @@ def load_exclusion(path):
exclusion list is normalized the same way so it can be matched against
the paths produced by `collect_inputs`.

Lines may be either a literal path or a glob pattern. A line containing
any of ``*``, ``?`` or ``[`` is expanded with `Path.glob` relative to the
same base directory; ``**`` matches zero or more directory levels, so
``.claude/**/*`` excludes every file and sub-directory below ``.claude``
at any depth. A literal path must point at an existing file, as before.
Comment on lines +180 to +184
Blank lines and lines starting with ``#`` are ignored.

Args:
path (str): Path to the exclusion file.
path (Path): Path to the exclusion file.

Returns:
tuple(list, bool): a list of files that are excluded from the copyright check and a boolean indicating whether
all paths listed in the exclusion file exist and are files.
tuple(set[str], bool): a de-duplicated set of paths (as str) that are
excluded from the copyright check, and a boolean
indicating whether every line resolved to
something: literal paths must exist and be files,
glob patterns must match at least one path.
"""

workspace_dir = Path(os.environ.get("BUILD_WORKSPACE_DIRECTORY", "").strip())

exclusion = []
exclusion: set[str] = set()
valid = True
with open(path, "r", encoding="utf-8") as file:
for item in file.read().splitlines():
if not item:
for line in file.read().splitlines():
item = line.strip()
if not item or item.startswith("#"):
continue
resolved = Path(workspace_dir / item)
if any(char in item for char in GLOB_CHARS):
Comment thread
MaximilianSoerenPollak marked this conversation as resolved.
try:
matches = {str(match) for match in workspace_dir.glob(item)}
except (ValueError, NotImplementedError) as err:
LOGGER.error("Invalid exclusion pattern %s: %s", item, err)
valid = False
continue
if not matches:
LOGGER.error("Exclusion pattern %s matched nothing.", item)
valid = False
continue
Comment on lines +212 to +215

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good finding. WIll include a test for it.

exclusion |= matches
continue
resolved = workspace_dir / item
if not resolved.exists():
LOGGER.error("Excluded file %s does not exist.", item)
valid = False
Expand All @@ -201,8 +224,7 @@ def load_exclusion(path):
LOGGER.error("Excluded file %s is not a file.", item)
valid = False
continue
exclusion.append(str(resolved))

exclusion.add(str(resolved))
LOGGER.debug(exclusion)
return exclusion, valid

Expand Down Expand Up @@ -573,7 +595,7 @@ def process_files(
files,
templates,
fix,
exclusion: list[str] | None = None,
exclusion: set[str] | None = None,
use_mmap=False,
encoding="utf-8",
): # pylint: disable=too-many-arguments
Expand All @@ -585,8 +607,8 @@ def process_files(
templates (dict): A dictionary where keys are file extensions
(e.g., '.py', '.txt') and values are strings or patterns
representing the required copyright text.
exclusion (list): A list of paths to files to be excluded from the copyright
check.
exclusion (set): A set of paths (as str) to files to be excluded from the
copyright check.
use_mmap (bool): Flag for using mmap function for reading files
(instead of standard option).
encoding (str): Encoding type to use when reading the file.
Expand All @@ -595,7 +617,7 @@ def process_files(
int: The number of files that do not contain the required copyright text.
"""
if exclusion is None:
exclusion = []
exclusion = set()
results = {"no_copyright": 0, "fixed": 0, "duplicate_copyright": 0}
for item in files:
name = Path(item).name
Expand Down Expand Up @@ -763,7 +785,7 @@ def main(argv=None):
LOGGER.error("Failed to load copyright text: %s", err)
return err.errno

exclusion = []
exclusion = set()
exclusion_valid = True
if args.exclusion_file:
try:
Expand Down