Report a replaced test process as terminated

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- The controller now calls `on_process_terminated` (JSON-RPC: `testProcessTerminated`) for a test process that it replaces with a new one. A test run replaces a pooled process when the environment content hash changed. It also replaces a process that has no endpoint, for example when the run starts after `terminate_test_process` but before the process exits. A restart that Revise asks for also replaces the process. The controller removed the old process without a report. The termination message of the old process then found no entry, so a consumer with a list of test processes kept the old entry.
+
 - On Julia 1.12 and later, a garbage collection in a test process no longer has to wait for the hang watchdog to wake up. The watchdog paces itself with a plain OS sleep, and that `ccall` kept its thread GC-unsafe for the full 50 ms poll interval, so every stop-the-world collection stalled until the sleep ended: 25–50 ms of extra time-to-safepoint per GC on Windows and 50–190 ms on GitHub's macOS runners, none of it counted in the GC time reported for a test item. The sleep now runs in a GC-safe region (`@ccall gc_safe=true`), which brings that delay to zero. On Windows it goes through libuv's `uv_sleep` rather than Win32 `Sleep`, because `@ccall` cannot express the `stdcall` convention `Sleep` needs on 32-bit Windows. Older Julia versions keep the previous sleep.
 
 - A test item that uses a `@testmodule` now runs with its own file path, so `@__DIR__` and `@__FILE__` inside it resolve against its own directory and its top-level frames are attributed to its own file. The test process evaluated the `@testmodule` into the same local that holds the test item's path, so the item's body was then handed the *setup's* path — an item in `test/sub/` declaring a setup defined in `test/` loaded `joinpath(@__DIR__, "data", …)` relative to `test/`, and a failure in it was reported at the setup's file. Only the first item to evaluate a given `@testmodule` on a process was affected, because later items reuse the already-evaluated module and never take that branch, which is why this presented as order- and scheduling-dependent. The working directory was always correct, so a bare relative path was unaffected. `TestItemRunner.jl`'s in-process runner never had this bug.

src/testitemcontroller.jl

@@ -2571,6 +2571,11 @@ function _replace_process_state!(
 
     delete!(c.test_processes, old_id)
 
+    # The later TestProcessTerminatedMsg of the old process finds no entry, so report it here.
+    if c.callbacks.on_process_terminated !== nothing
+        c.callbacks.on_process_terminated(old_id)
+    end
+
     return new_ps, old_id
 end
 

test/test_process_reuse.jl

@@ -168,6 +168,64 @@ end
     TestHelpers.timed_wait(controller_task, 600; label="restart-hash-change-controller")
 end
 
+@testitem "Process restart on env hash change reports the old process as terminated" setup=[TestHelpers] begin
+    # A consumer can keep its own list of test processes: it adds an id on
+    # `on_process_created` and removes the id on `on_process_terminated`. A restart
+    # replaces the process, so the controller must report the old id as terminated.
+    using TestItemControllers: TestItemController, TestRunItem, execute_testrun, shutdown, ControllerCallbacks
+    import UUIDs
+
+    pkg_path = joinpath(TestHelpers.TESTDATA_DIR, "BasicPackage")
+    discovered = TestHelpers.discover_test_items(pkg_path)
+    passing_items = filter(i -> i.label == "add works", discovered.items)
+
+    created_ids = String[]
+    terminated_ids = String[]
+    ids_lock = ReentrantLock()
+
+    callbacks = ControllerCallbacks(
+        on_testitem_started = (run_id, item_id, test_env_id) -> nothing,
+        on_testitem_passed = (run_id, item_id, test_env_id, duration) -> nothing,
+        on_testitem_failed = (run_id, item_id, test_env_id, messages, duration) -> nothing,
+        on_testitem_errored = (run_id, item_id, test_env_id, messages, duration) -> nothing,
+        on_testitem_skipped = (run_id, item_id, test_env_id) -> nothing,
+        on_append_output = (run_id, item_id, test_env_id, output) -> nothing,
+        on_attach_debugger = (run_id, pipe_name) -> nothing,
+        on_process_created = (id, test_env_id) -> lock(ids_lock) do
+            push!(created_ids, id)
+        end,
+        on_process_terminated = id -> lock(ids_lock) do
+            push!(terminated_ids, id)
+        end,
+    )
+
+    controller = TestItemController(callbacks; log_level=:Debug)
+
+    controller_task = @async try
+        run(controller)
+    catch err
+        @error "Controller error" exception=(err, catch_backtrace())
+    end
+
+    # The second run has a different env hash, so it replaces the process of the first run.
+    env_kwargs = TestHelpers._env_kwargs(discovered)
+    for env_content_hash in ("hash-A", "hash-B")
+        test_env = TestHelpers.make_test_environment(; env_kwargs..., env_content_hash)
+        work_units = [TestRunItem(item.id, test_env.id, nothing, :Debug) for item in passing_items]
+        execute_testrun(controller, string(UUIDs.uuid4()), [test_env], passing_items, work_units, discovered.setups, 1, nothing)
+    end
+
+    shutdown(controller)
+    TestHelpers.timed_wait(controller_task, 600; label="restart-reports-terminated-controller")
+
+    created, terminated = lock(ids_lock) do
+        copy(created_ids), copy(terminated_ids)
+    end
+    @test length(created) == 2
+    # After shutdown, the controller reports each process as terminated one time.
+    @test sort(terminated) == sort(created)
+end
+
 @testitem "Multiple consecutive restarts succeed" setup=[TestHelpers] begin
     # Regression test: pipe cleanup must work across many restart cycles,
     # not just the first one.