Repository: DRMacIver/shrinkray

Issue:
getting "Could not kill subprocess with pid {sp.pid}. Something has gone seriously wrong.", logic bug?
Hi David!

I'm shrinking input to Z3 that I feed it via a Python script. Z3 ignores `SIGINT` and continues, so when shrinkray tries to kill a process it really needs to use `SIGKILL`. However, I'm getting "Could not kill subprocess...", isn't the logic in `interrupt_wait_and_kill` simply wrong?:

```python
async def interrupt_wait_and_kill(sp: "trio.Process", delay: float = 0.1) -> None:
        ... # try to kill with SIGINT

        if sp.returncode is None:
            signal_group(sp, signal.SIGKILL)

        with trio.move_on_after(delay):
            await sp.wait()

        if sp.returncode is not None:
            raise ValueError(
                f"Could not kill subprocess with pid {sp.pid}. Something has gone seriously wrong."
            )
```

I think the last if should be `if sp.returncode is None:`, right?

I'm happy to open a PR if you tell me whether you also want a test for this situation, or whether you'd be happy simply with the fix.

Apart from that `shrinkray` is working great for my problem!

Discussion:

Patch:
diff --git a/src/shrinkray/__main__.py b/src/shrinkray/__main__.py
index 6035e10..921c2c0 100644
--- a/src/shrinkray/__main__.py
+++ b/src/shrinkray/__main__.py
@@ -73,7 +73,7 @@ async def interrupt_wait_and_kill(sp: "trio.Process", delay: float = 0.1) -> Non
         with trio.move_on_after(delay):
             await sp.wait()
 
-        if sp.returncode is not None:
+        if sp.returncode is None:
             raise ValueError(
                 f"Could not kill subprocess with pid {sp.pid}. Something has gone seriously wrong."
             )


Test patch:
diff --git a/tests/test_main.py b/tests/test_main.py
new file mode 100644
index 0000000..1fbb5f2
--- /dev/null
+++ b/tests/test_main.py
@@ -0,0 +1,32 @@
+import os
+import sys
+import trio
+import subprocess
+
+from shrinkray.__main__ import interrupt_wait_and_kill
+
+
+
+async def test_kill_process():
+    async with trio.open_nursery() as nursery:
+        kwargs = dict(
+            universal_newlines=False,
+            preexec_fn=os.setsid,
+            check=False,
+            stdout=subprocess.PIPE,
+        )
+        def call_with_kwargs(task_status=trio.TASK_STATUS_IGNORED):  # type: ignore
+            # start a subprocess that will just ignore SIGINT signals
+            return trio.run_process(
+                    [sys.executable,
+                     "-c",
+                     "import signal, sys, time; signal.signal(signal.SIGINT, lambda *a: 1); print(1); sys.stdout.flush(); time.sleep(1000)"],
+                    **kwargs, task_status=task_status)
+
+        sp = await nursery.start(call_with_kwargs)
+        line = await sp.stdout.receive_some(2)
+        assert line == b"1\n"
+        # must not raise ValueError but succeed at killing the process
+        await interrupt_wait_and_kill(sp)
+        assert sp.returncode is not None
+        assert sp.returncode != 0