Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/actions/repo-gate/repo_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,12 @@ def tracked(root: Path, exclude: list[str] | None = None) -> list[str]:
if exclude:
args += ["--", *(f":!{pattern}" for pattern in exclude)]
result = subprocess.run(args, capture_output=True, text=True, check=False)
# A failed command and a valid empty result would otherwise look the same to the caller.
# The command's own stderr (a bad pathspec, an unreadable root) is surfaced rather than dropped.
if result.returncode != 0 and result.stderr.strip():
print(f"git ls-files failed: {result.stderr.strip()}", file=sys.stderr)
if result.returncode != 0:
# A failed command's stdout is never trusted, even where it is non-empty.
# A partial listing read as complete is a scan that missed files and said nothing.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
reason = result.stderr.strip() or f"exit {result.returncode}, no stderr"
print(f"git ls-files failed: {reason}", file=sys.stderr)
return []
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
return [l for l in result.stdout.split("\n") if l]


Expand Down
22 changes: 22 additions & 0 deletions scripts/tests/test_repo_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,28 @@ def test_a_failed_ls_files_call_prints_gits_own_error(self) -> None:
self.assertIn("git ls-files failed", err.getvalue())
self.assertIn("Invalid pathspec magic", err.getvalue())

def test_a_failed_call_is_never_trusted_even_with_nonempty_stdout(self) -> None:
"""A partial listing read as a complete one is a scan that missed files silently."""
proc = subprocess.CompletedProcess([], 128, "kept.py\n", "fatal: something went wrong\n")
with (
mock.patch.object(repo_gate.subprocess, "run", return_value=proc),
contextlib.redirect_stderr(io.StringIO()),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
):
files = repo_gate.tracked(self.tmp, ["vendor/**"])
self.assertEqual([], files)

def test_a_failure_with_no_stderr_still_prints_a_reason(self) -> None:
"""Empty stderr on a nonzero exit must not read as a silent, unexplained empty scan."""
proc = subprocess.CompletedProcess([], 1, "", "")
with (
mock.patch.object(repo_gate.subprocess, "run", return_value=proc),
contextlib.redirect_stderr(io.StringIO()) as err,
):
files = repo_gate.tracked(self.tmp, ["vendor/**"])
self.assertEqual([], files)
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
self.assertIn("git ls-files failed", err.getvalue())
self.assertIn("exit 1", err.getvalue())


class TestHarness(unittest.TestCase):
def test_this_module_collects_a_plausible_number_of_cases(self) -> None:
Expand Down
Loading