diff --git a/actions/repo-hygiene/check-repo-hygiene.py b/actions/repo-hygiene/check-repo-hygiene.py index 0fb50aa..9051070 100755 --- a/actions/repo-hygiene/check-repo-hygiene.py +++ b/actions/repo-hygiene/check-repo-hygiene.py @@ -81,6 +81,13 @@ # evidenced exceptions rather than a way to quiet a genuine hit. NOT_A_TRACKER_KEY = re.compile(r"\b(?:ISO-\d+|RFC-\d+|SHA-\d+|UTF-\d+|AES-\d+|PEP-\d+|CVE-\d+|SMETS-\d+|FLEX-\d+|GPL-\d+|LGPL-\d+|AGPL-\d+)\b") +# Same shape at the end of a longer standard's name, matched against the line up to the key's +# end, since the key alone cannot see what comes before it: Unicode-DFS-2016, the SPDX name of +# Unicode's data files licence, which a Debian copyright file must spell exactly. Only that name +# is let through, so a project whose keys start DFS-, or a key that follows "Unicode-", is still +# reported. +PART_OF_A_STANDARD = re.compile(r"\bUnicode-DFS-\d+$") + # Any link into the internal Atlassian site, not only a wiki one. Review asked whether this # should require a /wiki/ path, since the name said "wiki link" while the pattern also matched a # Jira /browse/ URL. Widened the name rather than narrowing the pattern: a Jira board URL @@ -406,7 +413,9 @@ def check_no_internal_references(root, tracked, files): for number, line in enumerate(text.splitlines(), 1): for match in TRACKER_KEY.finditer(line): span = match.group(0) - if not NOT_A_TRACKER_KEY.search(span): + if not NOT_A_TRACKER_KEY.search(span) and not PART_OF_A_STANDARD.search( + line[: match.end()] + ): findings.append(f"{relative}:{number}: tracker key {span}") for match in INTERNAL_LINK.finditer(line): findings.append(f"{relative}:{number}: internal Atlassian link ({match.group(0)[:48]})") diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..9a3342d --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,15 @@ +"""Shared by the tests: the hygiene check, loaded from the action's script as it ships.""" + +import importlib.util +import pathlib + +ROOT = pathlib.Path(__file__).resolve().parent.parent + + +def load_hygiene(): + spec = importlib.util.spec_from_file_location( + "check_repo_hygiene", ROOT / "actions" / "repo-hygiene" / "check-repo-hygiene.py" + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module diff --git a/tests/test_internal_references.py b/tests/test_internal_references.py new file mode 100644 index 0000000..daa7164 --- /dev/null +++ b/tests/test_internal_references.py @@ -0,0 +1,36 @@ +"""The hygiene check's search for tracker keys: what it reports, and the standards it does not. + +The check is loaded from the action's script, so what is tested is the script that ships. The +keys it should report are assembled at run time, so that this file holds none for the check to +find. +""" + +import pytest +from conftest import load_hygiene + + +def findings(tmp_path, line): + path = tmp_path / "copyright" + path.write_text(line + "\n", encoding="utf-8") + return load_hygiene().check_no_internal_references(tmp_path, [path], [path]) + + +@pytest.mark.parametrize("line", [ + "License: Unicode-DFS-2016", + "License: GPL-3+ and Expat and Unicode-DFS-2016 and Unicode-3.0", + "Encoded as UTF-8, per RFC-3629 and ISO-8859", +]) +def test_an_external_standard_is_not_a_tracker_key(tmp_path, line): + assert findings(tmp_path, line) == [] + + +@pytest.mark.parametrize("before,key", [ + ("Fixes ", "XY" + "-22"), + ("See ", "ABC" + "-1"), + # Only the Unicode licence's name is let through, not every key ending its shape. + ("Fixes ", "DFS" + "-2016"), + ("MyUnicode-", "DFS" + "-2016"), + ("Unicode-", "XY" + "-22"), +]) +def test_a_tracker_key_is_reported(tmp_path, before, key): + assert findings(tmp_path, before + key) == [f"copyright:1: tracker key {key}"] diff --git a/tests/test_slack_workflows.py b/tests/test_slack_workflows.py index 2649d7b..6161e00 100644 --- a/tests/test_slack_workflows.py +++ b/tests/test_slack_workflows.py @@ -6,7 +6,6 @@ environment, and Slack itself. """ -import importlib.util import json import os import pathlib @@ -16,6 +15,7 @@ import pytest import yaml +from conftest import load_hygiene ROOT = pathlib.Path(__file__).resolve().parent.parent WORKFLOWS = ROOT / ".github" / "workflows" @@ -258,15 +258,6 @@ def test_the_token_reaches_curl_only_in_a_header_file(stubs, workflow, environme assert json.loads((stubs / "curl.stdin").read_text())["channel"] == channel -def hygiene(): - spec = importlib.util.spec_from_file_location( - "check_repo_hygiene", ROOT / "actions" / "repo-hygiene" / "check-repo-hygiene.py" - ) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - @pytest.mark.parametrize("uses", [ "L337-org/github-workflows/.github/workflows/slack-on-pull-request.yaml@" + "0" * 40, "L337-org/github-workflows/.github/workflows/slack-on-failure.yaml@" + "0" * 40, @@ -289,7 +280,7 @@ def test_hygiene_requires_v2_inputs_on_every_slack_call(uses, given, secrets, fo body["with"] = given if secrets is not None: body["secrets"] = secrets - findings = hygiene().slack_call_findings([(".github/workflows/x.yaml", {"jobs": {"slack": body}})]) + findings = load_hygiene().slack_call_findings([(".github/workflows/x.yaml", {"jobs": {"slack": body}})]) assert len(findings) == len(found) for finding, expected in zip(findings, found): assert expected in finding @@ -299,4 +290,4 @@ def test_every_shipped_caller_here_names_a_channel_and_passes_the_token(): documents = [ (p.name, yaml.safe_load(p.read_text())) for p in sorted(WORKFLOWS.glob("*.yaml")) ] - assert hygiene().slack_call_findings(documents) == [] + assert load_hygiene().slack_call_findings(documents) == []