Skip to content

Commit c2192eb

Browse files
committed
fix: isolate the real commit-check subprocess from repo and org config
The previous commit worked around one symptom (no git identity -> commit-check falls back to HEAD's author -> ignore_authors matches a bot -> the piped message check silently skips) by giving the test step a git identity in CI. That leaves TestRealCommitCheckBinary coupled to more than it should be about: it still runs from this repo's checkout, so it still picks up commit-check.toml, which still inherits the org's shared config over the network on every test run, and it still resolves ignore_authors and every other repo-level setting -- none of which the test's own docstring says it is about. Run it from an isolated, empty directory instead. commit-check reads cchk.toml/commit-check.toml only from its cwd (no upward search), so an empty cwd gets the built-in defaults on their own: no config file, no network fetch, no ignore_authors, and -- since git commands run from a non-repository directory return nothing -- no HEAD author to fall back to either. The test now only exercises what it says it exercises: the CLI's JSON contract for a piped message. This supersedes the git-identity workaround, which is no longer needed and is reverted here.
1 parent 8986da9 commit c2192eb

4 files changed

Lines changed: 28 additions & 21 deletions

File tree

‎.github/workflows/coverage.yml‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,6 @@ jobs:
1818
with:
1919
# 3.10 is the real floor, see test.yml
2020
python-version: "3.10"
21-
# See test.yml: TestRealCommitCheckBinary needs a non-ignored git
22-
# identity, or commit-check falls back to HEAD's own author (e.g.
23-
# dependabot[bot] on a dependency-bump PR) and silently skips.
24-
- name: Configure a git identity for the test suite
25-
run: |
26-
git config --global user.name "github-actions[bot]"
27-
git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com"
2821
- name: Install dependencies
2922
run: |
3023
pip install -r requirements.txt

‎.github/workflows/test.yml‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,6 @@ jobs:
3131
with:
3232
python-version: ${{ matrix.py }}
3333

34-
# actions/checkout sets no git identity. TestRealCommitCheckBinary pipes
35-
# a message to the real commit-check binary, unset from any commit --
36-
# with no local user.name, commit-check falls back to the checked-out
37-
# HEAD commit's own author to test against ignore_authors. On a
38-
# Dependabot PR that author is "dependabot[bot]", which the org's
39-
# inherited config ignores, so the check silently skips instead of
40-
# running. A neutral identity keeps the test about the piped message.
41-
- name: Configure a git identity for the test suite
42-
run: |
43-
git config --global user.name "github-actions[bot]"
44-
git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com"
45-
4634
- name: Install dependencies
4735
run: |
4836
python -m pip install --upgrade pip

‎main.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ def _relay_cli_notices(text: str) -> None:
511511

512512

513513
def run_check_json(
514-
args: list[str], input_text: str | None = None
514+
args: list[str], input_text: str | None = None, cwd: str | None = None
515515
) -> tuple[int, dict[str, Any] | None, str]:
516516
"""Run ``commit-check --format json`` and return (exit code, parsed JSON, raw output).
517517
@@ -526,6 +526,12 @@ def run_check_json(
526526
The parsed JSON is ``None`` when the CLI did not produce valid JSON; the
527527
raw output is kept so callers can fall back to showing it as text, and
528528
in that case it carries both streams so nothing the CLI said is lost.
529+
530+
``cwd`` picks the directory the CLI runs from -- left at ``None`` (the
531+
caller's own cwd) for the real action, where that directory is the
532+
checked-out repository on purpose. A caller that wants no config file,
533+
no ``ignore_authors``, and no ``git`` state to leak in (the unmocked
534+
binary test, notably) passes an isolated directory instead.
529535
"""
530536
command = ["commit-check", "--format", "json"] + args
531537
result = subprocess.run(
@@ -536,6 +542,7 @@ def run_check_json(
536542
text=True,
537543
encoding="utf-8",
538544
check=False,
545+
cwd=cwd,
539546
)
540547
out = result.stdout or ""
541548
err = result.stderr or ""

‎main_test.py‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2800,10 +2800,29 @@ class TestRealCommitCheckBinary(unittest.TestCase):
28002800
for every value. This is the one place that drift can fail a build. CI
28012801
installs requirements.txt, so the binary is always present there; the
28022802
skip only spares a contributor running the suite without it.
2803+
2804+
Run from an isolated, empty directory rather than the checked-out repo:
2805+
this repo's own ``commit-check.toml`` inherits the org's shared config
2806+
over the network, and a message piped in here is deliberately unrelated
2807+
to any real commit, so neither that config nor ``git``'s notion of the
2808+
current author (nor, through it, ``ignore_authors``) belongs in a test
2809+
about the CLI's JSON contract. A previous version of this test ran from
2810+
the repo as checked out, so it inherited both -- and, having no git
2811+
identity of its own, additionally fell back to HEAD's actual author to
2812+
weigh against ``ignore_authors``. On a Dependabot PR that author is
2813+
``dependabot[bot]``, which the org config ignores, so the message check
2814+
silently skipped instead of running, regardless of the message.
28032815
"""
28042816

2817+
def setUp(self):
2818+
tmpdir = tempfile.TemporaryDirectory()
2819+
self.addCleanup(tmpdir.cleanup)
2820+
self._cwd = tmpdir.name
2821+
28052822
def _run(self, message: str) -> tuple[int, dict]:
2806-
rc, data, raw = main.run_check_json(["--message"], input_text=message)
2823+
rc, data, raw = main.run_check_json(
2824+
["--message"], input_text=message, cwd=self._cwd
2825+
)
28072826
self.assertIsInstance(data, dict, f"CLI did not emit JSON:\n{raw}")
28082827
assert data is not None # for the type checker; asserted above
28092828
self.assertIn("checks", data)

0 commit comments

Comments
 (0)