From e74dbc62c1c9eeff6df03f026360535134ed3a78 Mon Sep 17 00:00:00 2001 From: Andy Woods <22278911+awoods187@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:19:54 -0400 Subject: [PATCH] Require an Artifacts section on PRs that change the CLI Unit tests mock the API client, so they check the CLI against the author's reading of the API, not the API itself. A new CI check fails PRs that touch src/ unless the description's Artifacts section holds real output (a code block) or a screenshot. The skip-artifacts label opts out for changes with nothing to run. The PR template and CONTRIBUTING.md explain what to include. --- .github/pull_request_template.md | 15 ++++++- .github/workflows/artifacts.yml | 20 +++++++++ CONTRIBUTING.md | 3 +- scripts/check_artifacts.py | 71 ++++++++++++++++++++++++++++++++ tests/test_check_artifacts.py | 69 +++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/artifacts.yml create mode 100755 scripts/check_artifacts.py create mode 100644 tests/test_check_artifacts.py diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 6f23a4d..c1c128b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -10,4 +10,17 @@ - [ ] Unit tests pass (`uv run pytest`) - [ ] Linter passes (`uv run ruff check .`) -- [ ] Manually tested with `dualentry ` +- [ ] Manually tested with `dualentry ` against a real API (see Artifacts) + +## Artifacts + + diff --git a/.github/workflows/artifacts.yml b/.github/workflows/artifacts.yml new file mode 100644 index 0000000..39db648 --- /dev/null +++ b/.github/workflows/artifacts.yml @@ -0,0 +1,20 @@ +name: Artifacts + +on: + pull_request: + branches: [main] + # `edited` re-runs the check when the description changes. + types: [opened, synchronize, reopened, edited, labeled, unlabeled] + +jobs: + artifacts: + if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-artifacts') }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + # Passed through env, never interpolated into the script. + - env: + PR_BODY: ${{ github.event.pull_request.body }} + run: python3 scripts/check_artifacts.py "origin/${{ github.base_ref }}" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b2aa08..eeef820 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,7 +53,8 @@ uv run pytest --cov=dualentry_cli --cov-report=term-missing 3. Ensure linting and tests pass 4. For user-facing changes, add a bullet under `## [Unreleased]` in `CHANGELOG.md`. CI fails PRs that touch `src/` without one; apply the `skip-changelog` label when the change isn't user-facing 5. If you touched dependencies, run `uv lock` and commit `uv.lock` -6. Open a PR against `main` +6. Run your change against a real API and paste the evidence under `## Artifacts` in the PR description: the commands and their output, JSON responses, before/after for writes, or screenshots of the result in the app. Unit tests mock the API client, so they can't show the CLI and the API agree. CI fails PRs that touch `src/` without a code block or screenshot there; apply the `skip-artifacts` label when there is nothing to run. Redact secrets and use test data, since this repository is public +7. Open a PR against `main` ## Releasing diff --git a/scripts/check_artifacts.py b/scripts/check_artifacts.py new file mode 100755 index 0000000..8a15376 --- /dev/null +++ b/scripts/check_artifacts.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +""" +Fail when a PR changes the CLI without showing it working against a real API. + +Unit tests mock the API client, so they check the CLI against the author's reading +of the API rather than the API itself. The Artifacts section is where the author +shows real runs: command output, JSON responses, screenshots of the result in the app. + +Usage: + PR_BODY="..." python scripts/check_artifacts.py # e.g. origin/main +""" + +from __future__ import annotations + +import os +import re +import sys + +from check_changelog import git, needs_note + +HEADING = re.compile(r"^##\s+Artifacts\s*$", re.MULTILINE | re.IGNORECASE) +NEXT_HEADING = re.compile(r"^#{1,2}\s", re.MULTILINE) +COMMENT = re.compile(r"", re.DOTALL) +# Evidence is captured output in a fenced block, or an uploaded image or video. +FENCED_BLOCK = re.compile(r"^```.*?\n.*?\S.*?^```", re.MULTILINE | re.DOTALL) +UPLOAD = re.compile(r"!\[[^\]]*\]\([^)]+\)| str | None: + """The Artifacts section's text without template comments, or None when there isn't one.""" + match = HEADING.search(body) + if match is None: + return None + rest = body[match.end() :] + end = NEXT_HEADING.search(rest) + return COMMENT.sub("", rest[: end.start()] if end else rest).strip() + + +def has_evidence(section: str) -> bool: + return bool(FENCED_BLOCK.search(section) or UPLOAD.search(section)) + + +def main() -> int: + if len(sys.argv) != 2: + print(__doc__) + return 2 + base = git("merge-base", sys.argv[1], "HEAD").strip() + + if not needs_note(git("diff", "--name-only", base, "HEAD").splitlines()): + print("No user-facing changes; artifacts not required.") + return 0 + + section = artifacts_section(os.environ.get("PR_BODY") or "") + if section is None: + print("This PR changes src/ but its description has no `## Artifacts` section.") + elif not has_evidence(section): + print("The `## Artifacts` section has no evidence: add command output in a ``` block, or a screenshot.") + else: + print("Artifacts found.") + return 0 + + print( + "Show the change working against a real API: the commands you ran and their output, " + "the JSON response, or a screenshot of the result in the app. Redact secrets first; " + "this repository is public. Apply the `skip-artifacts` label for changes with nothing to run.", + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_check_artifacts.py b/tests/test_check_artifacts.py new file mode 100644 index 0000000..03c3aea --- /dev/null +++ b/tests/test_check_artifacts.py @@ -0,0 +1,69 @@ +"""Tests for scripts/check_artifacts.py.""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +_SCRIPTS = Path(__file__).resolve().parent.parent / "scripts" +sys.path.insert(0, str(_SCRIPTS)) # check_artifacts imports check_changelog as a sibling +_SPEC = importlib.util.spec_from_file_location("check_artifacts", _SCRIPTS / "check_artifacts.py") +check = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(check) + +TEMPLATE = (Path(__file__).resolve().parent.parent / ".github" / "pull_request_template.md").read_text() + + +def test_untouched_template_has_an_empty_section(): + assert check.artifacts_section(TEMPLATE) == "" + assert not check.has_evidence(check.artifacts_section(TEMPLATE)) + + +def test_missing_heading(): + assert check.artifacts_section("## Summary\n\nDoes a thing.\n") is None + + +def test_section_stops_at_the_next_heading(): + body = "## Artifacts\n\nsee below\n\n## Merge note\n\n```\n$ dualentry x\n```\n" + assert check.artifacts_section(body) == "see below" + + +def test_subheadings_stay_in_the_section(): + body = "## Artifacts\n\n### Before\n\n```\n$ dualentry x\nError\n```\n" + assert check.has_evidence(check.artifacts_section(body)) + + +@pytest.mark.parametrize( + "section", + [ + "```\n$ dualentry invoices list\nShowing 3 of 359\n```", + "```bash\n$ dualentry webhooks get abc\n```", + "![webhook received](https://github.com/user-attachments/assets/abc123)", + '', + "https://github.com/user-attachments/assets/0f1e2d3c", + ], +) +def test_evidence(section): + assert check.has_evidence(section) + + +@pytest.mark.parametrize( + "section", + [ + "", + "N/A", + "Tested locally, works.", + "```\n```", + "```\n \n```", + "", + ], +) +def test_not_evidence(section): + assert not check.has_evidence(check.artifacts_section(f"## Artifacts\n\n{section}\n")) + + +def test_heading_match_is_case_insensitive(): + assert check.artifacts_section("## artifacts\n\n```\nok\n```\n") == "```\nok\n```"