diff --git a/.github/workflows/external-links.yml b/.github/workflows/external-links.yml index bbefa80..cc1f38b 100644 --- a/.github/workflows/external-links.yml +++ b/.github/workflows/external-links.yml @@ -50,25 +50,19 @@ jobs: set -euo pipefail python3 - <<'PY' import json, os + from scripts.check_external_links import render_issue_body, report_needs_issue report = json.load(open(os.environ["RUNNER_TEMP"] + "/report.json")) - lines = [f"The weekly external link check found {len(report['broken'])} broken link(s) among {report['checked']} checked " - f"({len(report['unknown'])} more could not be verified: blocked, rate limited or unreachable; " - f"{len(report['ignored'])} listed in `scripts/external-links-ignore.txt` were skipped).", ""] - for item in report["broken"]: - lines.append(f"- `{item['detail']}` {item['url']} (in {', '.join('`' + f + '`' for f in item['files'][:5])})") - lines += ["", "Find the current address of each source (or replace it), run `make build-explainers`, and this issue closes " - "itself on the next weekly run once nothing is broken. Reproduce locally with `python3 scripts/check_external_links.py`."] - open(os.environ["RUNNER_TEMP"] + "/body.md", "w").write("\n".join(lines) + "\n") - open(os.environ["RUNNER_TEMP"] + "/broken_count", "w").write(str(len(report["broken"]))) + open(os.environ["RUNNER_TEMP"] + "/body.md", "w").write(render_issue_body(report)) + open(os.environ["RUNNER_TEMP"] + "/report_needs_issue", "w").write("1" if report_needs_issue(report) else "0") PY existing="$(gh issue list --repo "$GITHUB_REPOSITORY" --state open --search "\"$ISSUE_TITLE\" in:title" --json number,title -q ".[] | select(.title == \"$ISSUE_TITLE\") | .number" | head -n 1)" - broken="$(cat "$RUNNER_TEMP/broken_count")" - if [ "$broken" -gt 0 ]; then + report_needs_issue="$(cat "$RUNNER_TEMP/report_needs_issue")" + if [ "$report_needs_issue" = "1" ]; then if [ -n "$existing" ]; then gh issue edit "$existing" --repo "$GITHUB_REPOSITORY" --body-file "$RUNNER_TEMP/body.md" else gh issue create --repo "$GITHUB_REPOSITORY" --title "$ISSUE_TITLE" --label documentation --body-file "$RUNNER_TEMP/body.md" fi elif [ -n "$existing" ]; then - gh issue close "$existing" --repo "$GITHUB_REPOSITORY" --reason completed --comment "The weekly check found no broken external links." + gh issue close "$existing" --repo "$GITHUB_REPOSITORY" --reason completed --comment "The weekly check found no broken links or stale ignore entries." fi diff --git a/scripts/check_external_links.py b/scripts/check_external_links.py index 74e73d3..92a961b 100644 --- a/scripts/check_external_links.py +++ b/scripts/check_external_links.py @@ -180,16 +180,19 @@ def run(urls: dict[str, list[str]], limit: int | None = None, workers: int = 8, ignore: dict[str, str] | None = None) -> dict: ignore = ignore or {} ignored = [{"url": url, "reason": ignore[url], "files": urls[url]} for url in sorted(urls) if url in ignore] + stale_ignored = [{"url": url, "reason": reason} for url, reason in sorted(ignore.items()) if url not in urls] urls = {url: files for url, files in urls.items() if url not in ignore} items = sorted(urls)[:limit] if limit else sorted(urls) results = {"ok": [], "broken": [], "unknown": []} with concurrent.futures.ThreadPoolExecutor(workers) as pool: for url, (kind, detail) in zip(items, pool.map(lambda u: check_url(u, timeout), items)): results[kind].append({"url": url, "detail": detail, "files": urls[url]}) - return {"checked": len(items), **{kind: results[kind] for kind in results}, "ignored": ignored} + return {"checked": len(items), **{kind: results[kind] for kind in results}, + "ignored": ignored, "stale_ignored": stale_ignored} def render(report: dict) -> str: + stale_ignored = report.get("stale_ignored", []) lines = [f"Checked {report['checked']} external links: {len(report['ok'])} ok, " f"{len(report['broken'])} broken, {len(report['unknown'])} unknown." + (f" {len(report['ignored'])} ignored." if report.get("ignored") else "")] @@ -201,9 +204,32 @@ def render(report: dict) -> str: if report.get("ignored"): lines += ["", "Ignored (listed in scripts/external-links-ignore.txt, not requested):"] lines += [f" {item['url']} ({item['reason']})" for item in report["ignored"]] + if stale_ignored: + lines += ["", f"Stale ignored URLs ({len(stale_ignored)} not found in checked documents):"] + lines += [f" {item['url']} ({item['reason']})" for item in stale_ignored] return "\n".join(lines) +def render_issue_body(report: dict) -> str: + stale_count = len(report.get("stale_ignored", [])) + stale_label = "entry" if stale_count == 1 else "entries" + lines = [f"The weekly external link check found {len(report['broken'])} broken link(s) among {report['checked']} checked " + f"({len(report['unknown'])} more could not be verified: blocked, rate limited or unreachable; " + f"{len(report['ignored'])} listed in `scripts/external-links-ignore.txt` were skipped). " + f"It also found {stale_count} stale ignore-list {stale_label} with no matching document.", ""] + for item in report["broken"]: + lines.append(f"- `{item['detail']}` {item['url']} (in {', '.join('`' + path + '`' for path in item['files'][:5])})") + lines += ["", "Fix any reported broken links and remove stale ignore-list entries, then run `make build-explainers`. " + "This issue closes itself on the next weekly run once both are clear. " + "Reproduce locally with `python3 scripts/check_external_links.py`."] + return "\n".join(lines) + "\n" + + +def report_needs_issue(report: dict) -> bool: + """Keep the weekly issue open while broken links or stale ignores remain.""" + return bool(report["broken"] or report.get("stale_ignored")) + + def main(argv=None) -> int: parser = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) parser.add_argument("paths", nargs="*", type=Path, help="markdown files to check (default: explainers/*.md and the top-level docs)") diff --git a/tests/test_check_external_links.py b/tests/test_check_external_links.py index cf8849c..2d20d0f 100644 --- a/tests/test_check_external_links.py +++ b/tests/test_check_external_links.py @@ -156,21 +156,35 @@ def test_main_reports_without_failing_unless_asked(server, tmp_path, capsys): capsys.readouterr() assert script.main([str(doc), "--timeout", "5", "--json", "--limit", "2", "--check-local"]) == 0 report = json.loads(capsys.readouterr().out) - assert report["checked"] == 2 and set(report) == {"checked", "ok", "broken", "unknown", "ignored"} + assert report["checked"] == 2 and set(report) == { + "checked", "ok", "broken", "unknown", "ignored", "stale_ignored" + } def test_ignore_list_skips_a_url_and_reports_it(server, tmp_path, capsys): doc = tmp_path / "doc.md" doc.write_text(f"[ok]({server}/ok) [gone]({server}/gone)\n", encoding="utf-8") ignore = tmp_path / "ignore.txt" - ignore.write_text(f"# a comment\n\n{server}/gone needs a login\n", encoding="utf-8") + ignore.write_text(f"# a comment\n\n{server}/gone needs a login\n{server}/retired no longer referenced\n", encoding="utf-8") assert script.main([str(doc), "--timeout", "5", "--check-local", "--ignore-file", str(ignore)]) == 0 out = capsys.readouterr().out assert "0 broken" in out and "1 ignored" in out and "(needs a login)" in out + assert "Stale ignored URLs (1 not found in checked documents)" in out + assert f"{server}/retired (no longer referenced)" in out assert script.main([str(doc), "--timeout", "5", "--check-local", "--json", "--ignore-file", str(ignore)]) == 0 report = json.loads(capsys.readouterr().out) assert report["checked"] == 1 and [item["url"] for item in report["ignored"]] == [f"{server}/gone"] assert report["ignored"][0]["files"] == [str(doc)] + assert report["stale_ignored"] == [{"url": f"{server}/retired", "reason": "no longer referenced"}] + assert "1 stale ignore-list entry" in script.render_issue_body(report) + assert "remove stale ignore-list entries" in script.render_issue_body(report) + assert script.report_needs_issue(report) + + +def test_weekly_issue_state_closes_only_when_broken_and_stale_lists_are_empty(): + assert script.report_needs_issue({"broken": [], "stale_ignored": []}) is False + assert script.report_needs_issue({"broken": [{"url": "https://example.org/gone"}], "stale_ignored": []}) is True + assert script.report_needs_issue({"broken": [], "stale_ignored": [{"url": "https://example.org/old"}]}) is True def test_ignore_list_requires_a_reason_and_tolerates_a_missing_file(tmp_path):