From d0ab67feeee4efb2480cdedb7cc7630144f36952 Mon Sep 17 00:00:00 2001
From: Simon Cropp <simon.cropp@gmail.com>
Date: Fri, 21 Aug 2026 21:24:07 +1000
Subject: [PATCH] Pin that killing a diff tool actually kills it

LaunchAndKill and LaunchAndKillAsync asserted that the tool was no longer
running after DiffRunner.Kill. That assertion cannot fail: FakeDiffTool sleeps
for five seconds and then exits by itself, and WaitForRunning polls for ten, so
"gone" is true whether the kill worked or did nothing whatsoever. Both tests
pass with ProcessCleanup.Kill short circuited to a bare return - I checked.

Assert on how the process ended rather than on whether it is still there.
WindowsProcess.TryTerminateProcess passes -1 to TerminateProcess, and a
FakeDiffTool that ran out its sleep returns 0, so the exit code separates the
two with no timing in it at all.

Reading it needs a handle opened before the process goes: Process.GetProcessById
holds none of its own, so OpenLaunched touches Handle while the tool is still
running. That is the same point ProcessEx makes on the tray side.

Neutering Kill now fails both tests on the exit code.
---
 src/DiffEngine.Tests/DiffRunnerTests.cs | 58 ++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 10 deletions(-)

diff --git a/src/DiffEngine.Tests/DiffRunnerTests.cs b/src/DiffEngine.Tests/DiffRunnerTests.cs
index ed9acacf..b278bc8f 100644
--- a/src/DiffEngine.Tests/DiffRunnerTests.cs
+++ b/src/DiffEngine.Tests/DiffRunnerTests.cs
@@ -175,10 +175,14 @@ public async Task LaunchAndKill()
         await WaitForRunning(true);
         await Assert.That(IsRunning()).IsTrue();
         await Assert.That(ProcessCleanup.IsRunning(command)).IsTrue();
+
+        using var launched = OpenLaunched();
         DiffRunner.Kill(file1, file2);
+
         await WaitForRunning(false);
         await Assert.That(IsRunning()).IsFalse();
         await Assert.That(ProcessCleanup.IsRunning(command)).IsFalse();
+        await AssertTerminated(launched);
     }
 
     [Test]
@@ -192,27 +196,61 @@ public async Task LaunchAndKillAsync()
         await WaitForRunning(true);
         await Assert.That(IsRunning()).IsTrue();
         await Assert.That(ProcessCleanup.IsRunning(command)).IsTrue();
+
+        using var launched = OpenLaunched();
         DiffRunner.Kill(file1, file2);
+
         await WaitForRunning(false);
         await Assert.That(IsRunning()).IsFalse();
         await Assert.That(ProcessCleanup.IsRunning(command)).IsFalse();
+        await AssertTerminated(launched);
+    }
+
+    /// <summary>
+    /// Opens a handle on the process the launch just started, before anything can kill it.
+    /// <para>
+    /// Process.GetProcessById holds no OS handle of its own, and a handle opened after the process
+    /// has gone cannot report how it went. Touching Handle here is what makes the exit code
+    /// readable afterwards.
+    /// </para>
+    /// </summary>
+    Process OpenLaunched()
+    {
+        var match = ProcessCleanup.FindAll().Single(_ => _.Command == Expected);
+        var process = Process.GetProcessById(match.Process);
+        _ = process.Handle;
+        return process;
+    }
+
+    /// <summary>
+    /// That the process was killed, rather than that it is merely gone.
+    /// <para>
+    /// The distinction is the whole point of this assertion. FakeDiffTool sleeps for five seconds
+    /// and then exits on its own, and WaitForRunning polls for ten, so "no longer running" is
+    /// true whether the kill worked or did nothing at all - these tests passed with
+    /// ProcessCleanup.Kill short circuited to a bare return. The exit code tells them apart:
+    /// WindowsProcess.TryTerminateProcess passes -1 to TerminateProcess, and a FakeDiffTool that
+    /// ran out its sleep returns 0.
+    /// </para>
+    /// </summary>
+    static async Task AssertTerminated(Process process)
+    {
+        await Assert.That(process.WaitForExit(5000)).IsTrue();
+        await Assert.That(process.ExitCode).IsNotEqualTo(0);
     }
 
     // Match this test's exact command, not any FakeDiffTool: DiffEngineTray.Tests
     // runs concurrently in the same CI job and launches its own FakeDiffTool
     // instances, which a machine-wide substring scan would see.
-    bool IsRunning()
-    {
-        var expected = command;
-        if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-        {
-            expected = expected.Replace("\"", "");
-        }
+    string Expected =>
+        RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
+            ? command
+            : command.Replace("\"", "");
 
-        return ProcessCleanup
+    bool IsRunning() =>
+        ProcessCleanup
             .FindAll()
-            .Any(_ => _.Command == expected);
-    }
+            .Any(_ => _.Command == Expected);
 
     // Process spawn and kill are asynchronous, so poll instead of guessing with a
     // fixed sleep. Also used at test start: the previous test's kill may still be