aider: Refactor run_unit_tests to kill the subprocess when it times out.

# Context:
USER: when `subprocess` `timeout` fires, do we need to kill the subprocess? just in `run_unit_tests`
ASSISTANT: When a subprocess times out, it is not automatically terminated. So, yes, if you want to ensure that the subprocess is terminated when it times out, you should explicitly kill it.

Here's how you can modify the `run_unit_tests` function to kill the subprocess when it times out:

```python
benchmark/benchmark.py
<<<<<<< ORIGINAL
        try:
            result = subprocess.run(
                command,
                stdout=subprocess.PIPE,
                stderr=subprocess.STDOUT,
                text=True,
                timeout=timeout,
            )
            if result.returncode != 0:
                all_tests_passed = False
                print(f"Test {test_file} failed")

            res = cleanup_test_output(result.stdout)

        except subprocess.TimeoutExpired:
            all_tests_passed = False
            res = f"Test {test_file} timed out after {timeout} seconds."
=======
        with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) as proc:
            try:
                stdout, _ = proc.communicate(timeout=timeout)
                if proc.returncode != 0:
                    all_tests_passed = False
                    print(f"Test {test_file} failed")
                res = cleanup_test_output(stdout)
            except subprocess.TimeoutExpired:
                proc.kill()
                proc.wait()
                all_tests_passed = False
                res = f"Test {test_file} timed out after {timeout} seconds."
>>>>>>> UPDATED
```

This change uses `subprocess.Popen` to create the subprocess and `communicate` to interact with it. If `communicate` times out, it raises `subprocess.TimeoutExpired`, and then we kill the subprocess with `proc.kill()`.
This commit is contained in:
Paul Gauthier 2023-06-28 18:29:58 -07:00
parent d1f0590911
commit 6632367978

View file

@ -374,23 +374,18 @@ def run_unit_tests(testdir, history_fname):
]
print(" ".join(command))
try:
result = subprocess.run(
command,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
timeout=timeout,
)
if result.returncode != 0:
with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) as proc:
try:
stdout, _ = proc.communicate(timeout=timeout)
if proc.returncode != 0:
all_tests_passed = False
print(f"Test {test_file} failed")
res = cleanup_test_output(stdout)
except subprocess.TimeoutExpired:
proc.kill()
proc.wait()
all_tests_passed = False
print(f"Test {test_file} failed")
res = cleanup_test_output(result.stdout)
except subprocess.TimeoutExpired:
all_tests_passed = False
res = f"Test {test_file} timed out after {timeout} seconds."
res = f"Test {test_file} timed out after {timeout} seconds."
with history_fname.open("a") as fh:
fh.write(f"```\n{res}\n```")