From 4d94630ac85b73a1eb9d03a3b59589feafe6113a Mon Sep 17 00:00:00 2001 From: Gavin Lucas Date: Fri, 23 Oct 2026 08:33:54 +0100 Subject: [PATCH 1/4] Do not report the Unicode data files licence name as a tracker key Unicode-DFS-2016 is the SPDX name of the licence Unicode's data files are under, and a Debian copyright file for a binary that links crates carrying Unicode tables must name it exactly. Its DFS-2016 has a tracker key's shape, so the check failed every pull request touching such a file. A match cannot see what comes before it, so adding DFS-\d+ to the list of external standards would also have let through every key of a project whose prefix is DFS. The check instead reads the text just before the match, and lets it through only after "Unicode-". New tests load the shipped script and check that the licence name and three of the existing exceptions are not reported, and that a key - DFS-2016 itself, alone or after a word ending in Unicode - still is. The keys are assembled at run time, so the file holds none for the check to find. The two licence cases fail without the change. The script's loader moves to conftest.py, shared with the Slack tests. Signed-off-by: Gavin Lucas --- actions/repo-hygiene/check-repo-hygiene.py | 10 +++++- tests/conftest.py | 15 +++++++++ tests/test_internal_references.py | 36 ++++++++++++++++++++++ tests/test_slack_workflows.py | 9 ++---- 4 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 tests/conftest.py create mode 100644 tests/test_internal_references.py diff --git a/actions/repo-hygiene/check-repo-hygiene.py b/actions/repo-hygiene/check-repo-hygiene.py index 0fb50aa..aee5079 100755 --- a/actions/repo-hygiene/check-repo-hygiene.py +++ b/actions/repo-hygiene/check-repo-hygiene.py @@ -81,6 +81,12 @@ # 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, read from what comes just before it, since +# a match cannot see past its own start: 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- is still reported. +PART_OF_A_STANDARD = re.compile(r"\bUnicode-$") + # 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 +412,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.start()] + ): 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..0e94f45 --- /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"), +]) +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..3d1a105 100644 --- a/tests/test_slack_workflows.py +++ b/tests/test_slack_workflows.py @@ -17,6 +17,8 @@ import pytest import yaml +from conftest import load_hygiene + ROOT = pathlib.Path(__file__).resolve().parent.parent WORKFLOWS = ROOT / ".github" / "workflows" TOKEN = "xoxb-test-" + os.urandom(8).hex() @@ -259,12 +261,7 @@ def test_the_token_reaches_curl_only_in_a_header_file(stubs, workflow, environme 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 + return load_hygiene() @pytest.mark.parametrize("uses", [ From 2991357410a416c6efb980b75f5cfb73e3d56a5c Mon Sep 17 00:00:00 2001 From: Gavin Lucas Date: Fri, 23 Oct 2026 08:35:57 +0100 Subject: [PATCH 2/4] Call the shared hygiene loader directly in the Slack tests Moving the loader to conftest.py left an unused importlib import and a one-line wrapper around it. Signed-off-by: Gavin Lucas --- tests/test_slack_workflows.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/test_slack_workflows.py b/tests/test_slack_workflows.py index 3d1a105..25d793e 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 @@ -260,10 +259,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(): - return load_hygiene() - - @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, @@ -286,7 +281,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 @@ -296,4 +291,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) == [] From ccc62914f53dc14c4504a67c1d183b7fa8d7ab18 Mon Sep 17 00:00:00 2001 From: Gavin Lucas Date: Fri, 23 Oct 2026 08:37:10 +0100 Subject: [PATCH 3/4] Sort the tests' imports as ruff's default isort rule does Signed-off-by: Gavin Lucas --- tests/test_internal_references.py | 1 - tests/test_slack_workflows.py | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/test_internal_references.py b/tests/test_internal_references.py index 0e94f45..0a5a9e5 100644 --- a/tests/test_internal_references.py +++ b/tests/test_internal_references.py @@ -6,7 +6,6 @@ """ import pytest - from conftest import load_hygiene diff --git a/tests/test_slack_workflows.py b/tests/test_slack_workflows.py index 25d793e..6161e00 100644 --- a/tests/test_slack_workflows.py +++ b/tests/test_slack_workflows.py @@ -15,7 +15,6 @@ import pytest import yaml - from conftest import load_hygiene ROOT = pathlib.Path(__file__).resolve().parent.parent From 1f6229c8cc09740709df9be248cf8dba9e10bc6a Mon Sep 17 00:00:00 2001 From: Gavin Lucas Date: Fri, 23 Oct 2026 08:38:09 +0100 Subject: [PATCH 4/4] Let through only the Unicode licence name, not any key after Unicode- The exception read only what came before a key, so Unicode-XY-22 would have been let through as well as Unicode-DFS-2016, though the comment said only that name was. It now matches the line up to the key's end against Unicode-DFS- and a number, and a test of a key after Unicode- fails without the change. Signed-off-by: Gavin Lucas --- actions/repo-hygiene/check-repo-hygiene.py | 13 +++++++------ tests/test_internal_references.py | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/actions/repo-hygiene/check-repo-hygiene.py b/actions/repo-hygiene/check-repo-hygiene.py index aee5079..9051070 100755 --- a/actions/repo-hygiene/check-repo-hygiene.py +++ b/actions/repo-hygiene/check-repo-hygiene.py @@ -81,11 +81,12 @@ # 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, read from what comes just before it, since -# a match cannot see past its own start: 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- is still reported. -PART_OF_A_STANDARD = re.compile(r"\bUnicode-$") +# 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 @@ -413,7 +414,7 @@ def check_no_internal_references(root, tracked, files): for match in TRACKER_KEY.finditer(line): span = match.group(0) if not NOT_A_TRACKER_KEY.search(span) and not PART_OF_A_STANDARD.search( - line[: match.start()] + line[: match.end()] ): findings.append(f"{relative}:{number}: tracker key {span}") for match in INTERNAL_LINK.finditer(line): diff --git a/tests/test_internal_references.py b/tests/test_internal_references.py index 0a5a9e5..daa7164 100644 --- a/tests/test_internal_references.py +++ b/tests/test_internal_references.py @@ -30,6 +30,7 @@ def test_an_external_standard_is_not_a_tracker_key(tmp_path, line): # 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}"]