Skip to content

Commit 928330f

Browse files
committed
run-task: clean up subprocess usage in _clean_git_checkout
This was copied from `run_command` in 8d7d8b1, but some stuff doesn't make sense here: - we don't stream the output to our log so we don't need to disable buffering or go through a TextIOWrapper - we don't want to pass our stdin to the command Instead we can use subprocess.run and let it capture stdout and check the returncode.
1 parent 3800fe2 commit 928330f

File tree

2 files changed

+12
-24
lines changed

2 files changed

+12
-24
lines changed

src/taskgraph/run-task/run-task

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -597,26 +597,18 @@ def _clean_git_checkout(destination_path):
597597
"-nxdff",
598598
]
599599
print_line(b"vcs", b"executing %r\n" % args)
600-
p = subprocess.Popen(
600+
p = subprocess.run(
601601
args,
602-
# Disable buffering because we want to receive output
603-
# as it is generated so timestamps in logs are
604-
# accurate.
605-
bufsize=0,
606602
stdout=subprocess.PIPE,
607-
stdin=sys.stdin.fileno(),
603+
encoding="latin1",
608604
cwd=destination_path,
609605
env=os.environ,
606+
check=True,
610607
)
611-
stdout = io.TextIOWrapper(p.stdout, encoding="latin1")
612-
ret = p.wait()
613-
if ret:
614-
sys.exit(ret)
615-
data = stdout.read()
616608
prefix = "Would remove "
617609
filenames = [
618610
os.path.join(destination_path, line[len(prefix) :])
619-
for line in data.splitlines()
611+
for line in p.stdout.splitlines()
620612
]
621613
print_line(b"vcs", b"removing %r\n" % filenames)
622614
for filename in filenames:

test/test_scripts_run_task.py

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import functools
2-
import io
32
import os
43
import site
54
import stat
@@ -293,23 +292,20 @@ def test_clean_git_checkout(monkeypatch, mock_stdin, run_task_mod):
293292
output_str = (
294293
f"{prefix}{untracked_dir_rel_path}/\n{prefix}{untracked_file_rel_path}\n"
295294
)
296-
output_bytes = output_str.encode("latin1")
297-
output = io.BytesIO(output_bytes)
295+
296+
real_popen = subprocess.Popen
298297

299298
def _Popen(
300299
args,
301-
bufsize=None,
302-
stdout=None,
303-
stderr=None,
304-
stdin=None,
305-
cwd=None,
306-
env=None,
307300
**kwargs,
308301
):
309-
return Mock(
310-
stdout=output,
311-
wait=lambda: 0,
302+
kwargs["stdin"] = subprocess.PIPE
303+
proc = real_popen(
304+
["cat"],
305+
**kwargs,
312306
)
307+
proc.stdin.write(output_str)
308+
return proc
313309

314310
monkeypatch.setattr(
315311
subprocess,

0 commit comments

Comments
 (0)