Skip to content

Commit ec06cd9

Browse files
committed
refactor: trim git preflight comments and tests
Keep the behavior, cut the prose: shorter module and test docstrings, one-line comments in plain_modules.py and .flake8, and a leaner test file covering the same two failure modes. Also strip ANSI codes before asserting on CLI output, which fixes the wrapped broken-git assertions.
1 parent 5c34b57 commit ec06cd9

4 files changed

Lines changed: 40 additions & 180 deletions

File tree

‎.flake8‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ exclude =
2020
per-file-ignores =
2121
__init__.py: F401
2222
tests/*: U100
23-
# plain_modules must call git_preflight.require_git() before importing GitPython,
24-
# which raises from module scope when git is missing or broken. That call is a
25-
# statement between imports, so every import below it trips E402.
23+
# require_git() runs between imports in plain_modules.py, so the imports below it trip E402
2624
plain_modules.py: E402
2725
max-complexity = 15
2826
enable-extensions = U100

‎git_preflight.py‎

Lines changed: 12 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,77 +1,41 @@
11
"""Verify a working git is available before GitPython is imported.
22
3-
GitPython probes for the git executable when it is first imported and raises from
4-
module scope if it is missing or broken. That happens while plain2code is still
5-
importing its own modules, so a check inside main() cannot intercept it -- the
6-
user gets a raw traceback instead of an explanation.
7-
8-
Two failure modes need separate handling. A missing git is simply absent from
9-
PATH. A broken git resolves but cannot run: on macOS without the Command Line
10-
Tools /usr/bin/git is a stub that exits non-zero, and stale shims behave the same
11-
way. GIT_PYTHON_REFRESH=quiet does not help with the latter, because GitPython
12-
only consults that setting for a git it cannot find. A PATH lookup alone is
13-
therefore not sufficient, so this module runs ``git version`` -- mirroring the
14-
``git_available`` check in install/bash/install.sh.
3+
GitPython probes for the git executable on import and raises from module scope
4+
if it is missing or broken, so the CLI would crash with a traceback before
5+
main() could explain it.
156
"""
167

17-
from __future__ import annotations
18-
198
import shutil
209
import subprocess
2110
import sys
2211

2312
from plain2code_console import console
2413

25-
# Long enough for a cold-cache process spawn, short enough that a wedged shim
26-
# cannot hang the CLI indefinitely.
27-
GIT_VERSION_TIMEOUT_SECONDS = 10
28-
2914
GIT_MISSING_MESSAGE = "git is not installed. Please install git and try again."
3015
GIT_BROKEN_MESSAGE = "git is installed but not working. Please repair your git installation and try again."
3116
MACOS_BROKEN_GIT_HINT = (
3217
"This usually means the Command Line Tools are missing; install them with 'xcode-select --install'."
3318
)
3419

3520

36-
def broken_git_message() -> str:
37-
"""The broken-git message, with a platform hint where there is a likely cause."""
38-
if sys.platform == "darwin":
39-
return f"{GIT_BROKEN_MESSAGE}\n{MACOS_BROKEN_GIT_HINT}"
40-
41-
return GIT_BROKEN_MESSAGE
42-
43-
44-
def find_git() -> str | None:
45-
"""Return the path to the git executable, or None if it is not on PATH."""
46-
return shutil.which("git")
47-
48-
49-
def git_runs(git_path: str) -> bool:
50-
"""Whether ``git version`` actually succeeds for the given executable.
51-
52-
A resolvable name can still be unusable, so the command is run rather than
53-
assumed. Any failure to execute it at all counts as unusable.
54-
"""
21+
def git_runs(git_path):
22+
"""Whether `git version` succeeds. A git on PATH can still be unusable: on
23+
macOS without the Command Line Tools /usr/bin/git is a stub that exits with
24+
an xcrun error, and GIT_PYTHON_REFRESH=quiet does not suppress that."""
5525
try:
56-
result = subprocess.run(
57-
[git_path, "version"],
58-
capture_output=True,
59-
timeout=GIT_VERSION_TIMEOUT_SECONDS,
60-
)
26+
return subprocess.run([git_path, "version"], capture_output=True, timeout=10).returncode == 0
6127
except (OSError, subprocess.SubprocessError):
6228
return False
6329

64-
return result.returncode == 0
6530

66-
67-
def require_git() -> None:
31+
def require_git():
6832
"""Exit with a clean message unless a working git is available."""
69-
git_path = find_git()
70-
33+
git_path = shutil.which("git")
7134
if git_path is None:
7235
console.error(f"{GIT_MISSING_MESSAGE}\n")
7336
sys.exit(1)
7437

7538
if not git_runs(git_path):
76-
console.error(f"{broken_git_message()}\n")
39+
hint = f"\n{MACOS_BROKEN_GIT_HINT}" if sys.platform == "darwin" else ""
40+
console.error(f"{GIT_BROKEN_MESSAGE}{hint}\n")
7741
sys.exit(1)

‎plain_modules.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,14 @@
66
import zipfile
77
from functools import cached_property
88

9-
# GitPython probes for the git executable when it is first imported and reports a
10-
# missing or broken one by raising from module scope, which would crash the CLI
11-
# with a traceback before main() could explain it. Diagnose git ourselves first.
129
from git_preflight import require_git
1310
from plain2code_exceptions import (
1411
InvalidModuleArchiveError,
1512
MissingPreviousFunctionalitiesError,
1613
ModuleDoesNotExistError,
1714
)
1815

16+
# Must run before GitPython is imported, which raises on a missing or broken git.
1917
require_git()
2018

2119
from git import Repo

‎tests/test_git_preflight.py‎

Lines changed: 26 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,40 @@
11
"""Regression tests for issue #133.
22
3-
When git is unusable the CLI must report a clean, one-line diagnosis and exit
4-
non-zero -- never dump a Python traceback.
5-
6-
The failure is import-time: GitPython probes for the git executable when it is
7-
first imported and raises from module scope, before ``main()`` runs. A check
8-
inside ``main()`` cannot intercept it, so these tests run the CLI in a
9-
subprocess and assert on what a user would actually see. Two failure modes are
10-
covered, because they surface through different GitPython paths:
11-
12-
- git missing entirely -- nothing named ``git`` on PATH.
13-
- git present but broken -- a ``git`` that resolves but exits non-zero, as on
14-
macOS without the Command Line Tools. ``GIT_PYTHON_REFRESH=quiet`` does not
15-
suppress this one, which is why a PATH lookup alone is not a sufficient check.
16-
17-
``sys.executable`` is launched by absolute path, so replacing PATH does not stop
18-
Python from starting -- it only controls which git, if any, can be found.
3+
When git is unusable the CLI must report a clean message and exit 1, never dump
4+
a traceback. The failure is at import time (GitPython raises from module scope
5+
before main() runs), so the CLI is run in a subprocess and asserted on what a
6+
user would actually see. sys.executable is launched by absolute path, so
7+
replacing PATH only controls which git, if any, can be found.
198
"""
209

2110
import os
11+
import re
2212
import subprocess
2313
import sys
2414
from pathlib import Path
2515

2616
import pytest
2717

28-
from git_preflight import (
29-
GIT_BROKEN_MESSAGE,
30-
GIT_MISSING_MESSAGE,
31-
MACOS_BROKEN_GIT_HINT,
32-
broken_git_message,
33-
git_runs,
34-
)
18+
from git_preflight import GIT_BROKEN_MESSAGE, GIT_MISSING_MESSAGE
3519

3620
REPO_ROOT = Path(__file__).resolve().parents[1]
37-
3821
TRACEBACK_MARKER = "Traceback (most recent call last)"
3922

40-
# Every entry point, including the ones that do no git work themselves: the crash
41-
# was in the import chain, so it hit them all equally.
42-
CLI_INVOCATIONS = [
43-
pytest.param(["--version"], id="version"),
44-
pytest.param(["--status"], id="status"),
45-
pytest.param(["does-not-exist.plain"], id="render"),
46-
]
47-
23+
# The crash was in the import chain, so it hit every entry point equally.
24+
CLI_INVOCATIONS = [["--version"], ["--status"], ["does-not-exist.plain"]]
4825

49-
def _env(path_dir=None):
50-
"""The current environment, with PATH optionally replaced.
5126

52-
GitPython overrides are dropped so the PATH search is the only thing
53-
deciding whether git is found.
54-
"""
27+
def run_cli(args, path_dir=None):
28+
"""Run the CLI and return its exit code and output, stripped of styling and
29+
with whitespace collapsed, so assertions survive console line wrapping."""
5530
env = os.environ.copy()
31+
# Drop GitPython overrides so the PATH search decides whether git is found.
5632
env.pop("GIT_PYTHON_GIT_EXECUTABLE", None)
5733
env.pop("GIT_PYTHON_REFRESH", None)
34+
env.setdefault("CODEPLAIN_API_KEY", "dummy")
5835
if path_dir is not None:
5936
env["PATH"] = str(path_dir)
60-
env.setdefault("CODEPLAIN_API_KEY", "dummy")
61-
return env
62-
6337

64-
def _run_cli(args, env):
6538
result = subprocess.run(
6639
[sys.executable, "plain2code.py", *args],
6740
cwd=REPO_ROOT,
@@ -70,12 +43,8 @@ def _run_cli(args, env):
7043
text=True,
7144
timeout=120,
7245
)
73-
return result.returncode, result.stdout + result.stderr
74-
75-
76-
def _normalize(output):
77-
"""Collapse whitespace so assertions survive console line wrapping."""
78-
return " ".join(output.split())
46+
output = re.sub(r"\x1b\[[0-9;]*m", "", result.stdout + result.stderr)
47+
return result.returncode, " ".join(output.split())
7948

8049

8150
@pytest.fixture
@@ -89,15 +58,11 @@ def no_git_dir(tmp_path):
8958
@pytest.fixture
9059
def broken_git_dir(tmp_path):
9160
"""A directory whose git resolves but always fails, used as the whole PATH.
92-
93-
This is the shape of macOS without the Command Line Tools: the name is
94-
there, running it is not possible.
95-
"""
61+
This is the shape of macOS without the Command Line Tools."""
9662
d = tmp_path / "broken-git"
9763
d.mkdir()
9864
if sys.platform == "win32":
99-
git = d / "git.bat"
100-
git.write_text("@echo off\nexit /b 127\n")
65+
(d / "git.bat").write_text("@echo off\nexit /b 127\n")
10166
else:
10267
git = d / "git"
10368
git.write_text("#!/bin/sh\nexit 127\n")
@@ -107,92 +72,27 @@ def broken_git_dir(tmp_path):
10772

10873
@pytest.mark.parametrize("args", CLI_INVOCATIONS)
10974
def test_missing_git_reports_cleanly(args, no_git_dir):
110-
rc, output = _run_cli(args, _env(no_git_dir))
75+
rc, output = run_cli(args, no_git_dir)
11176

112-
assert GIT_MISSING_MESSAGE in _normalize(output), output
77+
assert GIT_MISSING_MESSAGE in output, output
11378
assert TRACEBACK_MARKER not in output, output
79+
# GitPython's own advice about GIT_PYTHON_REFRESH must not reach the user.
80+
assert "GIT_PYTHON_REFRESH" not in output, output
11481
assert rc == 1, output
11582

11683

11784
@pytest.mark.parametrize("args", CLI_INVOCATIONS)
11885
def test_broken_git_reports_cleanly(args, broken_git_dir):
119-
# A git that resolves but cannot run must be diagnosed as broken rather than
120-
# missing -- and must not reach GitPython's import-time traceback.
121-
rc, output = _run_cli(args, _env(broken_git_dir))
122-
normalized = _normalize(output)
86+
rc, output = run_cli(args, broken_git_dir)
12387

124-
assert GIT_BROKEN_MESSAGE in normalized, output
125-
assert GIT_MISSING_MESSAGE not in normalized, output
88+
assert GIT_BROKEN_MESSAGE in output, output
89+
assert GIT_MISSING_MESSAGE not in output, output
12690
assert TRACEBACK_MARKER not in output, output
12791
assert rc == 1, output
12892

12993

130-
@pytest.mark.parametrize("args", CLI_INVOCATIONS)
131-
def test_missing_git_does_not_leak_gitpython_advice(args, no_git_dir):
132-
# GitPython's own message coaches the user about $GIT_PYTHON_REFRESH and
133-
# git.refresh(), which are irrelevant to someone who just needs to install
134-
# git. None of it should reach the user.
135-
_, output = _run_cli(args, _env(no_git_dir))
136-
137-
assert "GIT_PYTHON_REFRESH" not in output, output
138-
assert "Bad git executable" not in output, output
139-
140-
141-
# plain_modules is deliberately not tested as a standalone import: it has a
142-
# pre-existing circular import with file_utils that fails whether or not git is
143-
# present, so the assertion would hold for the wrong reason.
144-
@pytest.mark.parametrize("module", ["plain2code", "file_utils"])
145-
@pytest.mark.parametrize("path_fixture", ["no_git_dir", "broken_git_dir"])
146-
def test_importing_git_dependent_module_exits_cleanly(module, path_fixture, request):
147-
"""Importing a module in the CLI's git-dependent import chain must diagnose
148-
git rather than traceback."""
149-
path_dir = request.getfixturevalue(path_fixture)
150-
result = subprocess.run(
151-
[sys.executable, "-c", f"import {module}"],
152-
cwd=REPO_ROOT,
153-
env=_env(path_dir),
154-
capture_output=True,
155-
text=True,
156-
timeout=120,
157-
)
158-
output = result.stdout + result.stderr
159-
160-
assert TRACEBACK_MARKER not in output, output
161-
assert result.returncode == 1, output
162-
163-
16494
def test_cli_works_when_git_is_present():
165-
# The check must not get in the way of normal use.
166-
rc, output = _run_cli(["--version"], _env())
95+
rc, output = run_cli(["--version"])
16796

16897
assert rc == 0, output
16998
assert "codeplain version" in output, output
170-
171-
172-
def test_git_runs_accepts_real_git():
173-
import shutil
174-
175-
git_path = shutil.which("git")
176-
assert git_path is not None, "test environment has no git"
177-
assert git_runs(git_path) is True
178-
179-
180-
def test_git_runs_rejects_failing_git(broken_git_dir):
181-
broken = broken_git_dir / ("git.bat" if sys.platform == "win32" else "git")
182-
183-
assert git_runs(str(broken)) is False
184-
185-
186-
def test_git_runs_rejects_nonexistent_path(tmp_path):
187-
# OSError from exec must be treated as unusable, not propagated.
188-
assert git_runs(str(tmp_path / "definitely-not-here")) is False
189-
190-
191-
@pytest.mark.skipif(sys.platform != "darwin", reason="hint is macOS-specific")
192-
def test_broken_git_message_hints_at_command_line_tools_on_macos():
193-
assert MACOS_BROKEN_GIT_HINT in broken_git_message()
194-
195-
196-
@pytest.mark.skipif(sys.platform == "darwin", reason="hint is macOS-specific")
197-
def test_broken_git_message_has_no_macos_hint_elsewhere():
198-
assert broken_git_message() == GIT_BROKEN_MESSAGE

0 commit comments

Comments
 (0)