From 0a8ed47bb6fddefd2d4706bd1ee3f8c51d267e81 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Wed, 30 Sep 2026 15:24:15 +0200 Subject: [PATCH] Report a replaced test process as terminated _replace_process_state! removes the old process id from c.test_processes, but it does not call on_process_terminated. The TestProcessTerminatedMsg that the IO task of the old process posts later finds no entry, so its handler does not call the callback either. A consumer that keeps its own list of test processes, for example TestItemRuns, then keeps the id of a process that does not exist. A test run replaces a pooled process when the env content hash changed, or when the process has no endpoint because a termination started before the run. The Revise restart and the IO-error restart use the same function. _replace_process_state! now calls on_process_terminated(old_id) after it removes the old id. The callers report the new id with on_process_created after this. --- CHANGELOG.md | 2 ++ src/testitemcontroller.jl | 5 ++++ test/test_process_reuse.jl | 58 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72da68e..046b4ae 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/src/testitemcontroller.jl b/src/testitemcontroller.jl index 55b252d..9de2750 100644 --- a/src/testitemcontroller.jl +++ b/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 diff --git a/test/test_process_reuse.jl b/test/test_process_reuse.jl index 0fb86a5..7e07bba 100644 --- a/test/test_process_reuse.jl +++ b/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.