Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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.
Resolved
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved
if result.stderr.strip():
print(f"git ls-files failed: {result.stderr.strip()}", file=sys.stderr)
return []
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved
return [l for l in result.stdout.split("\n") if l]


Expand Down
7 changes: 7 additions & 0 deletions scripts/tests/test_repo_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,13 @@ 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):
files = repo_gate.tracked(self.tmp, ["vendor/**"])
self.assertEqual([], files)
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved


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