Skip to content

Report a replaced test process as terminated - #126

Open
disberd wants to merge 1 commit into
julia-testitems:mainfrom
disberd:fix/replace-process-notifies-terminated
Open

disberd wants to merge 1 commit into
julia-testitems:mainfrom
disberd:fix/replace-process-notifies-terminated

Conversation

@disberd

@disberd disberd commented Sep 30, 2026 •

Copy link
Copy Markdown

I found this problem while I was trying out a customized version of the JuliaMCP.jl server. I used AI help to debug the problem to its root cause and to prepare the fix.

Problem

The docstring of terminate_test_process says that on_process_terminated fires when the process ends. This does not happen when a test run replaces a pooled process with a new one. A consumer that keeps its own list of test processes then keeps the old id. TestItemRuns is one such consumer: it adds an entry to session.processes on on_process_created and removes it only on on_process_terminated. Its process list then shows a process that does not exist, and terminate_test_process on that id only logs Ignoring terminate request for unknown process.

Reproduction:

  1. Run a test item with env_content_hash = "hash-A".
  2. Run it again on the same controller with env_content_hash = "hash-B".
  3. Shut down the controller.

on_process_created reports two ids. on_process_terminated reports only the second id.

A run that starts after terminate_test_process on an idle process, but before the process exits, gives the same result. A restart that Revise asks for gives it too.

Cause

handle!(::GetProcsForTestRunMsg) restarts a reused process when the process has no endpoint or when the env content hash changed (src/testitemcontroller.jl:789-808). The restart calls _replace_process_state! (src/testitemcontroller.jl:2499-2575). That function makes a new process, kills the old one and removes the old id from c.test_processes (:2572). It does not call on_process_terminated. The IO task of the old process then posts TestProcessTerminatedMsg, but its handler finds no entry for the id (:652) and calls no callback. The Revise restart (:2054) and the IO-error restart (:2239) use the same function.

Fix

_replace_process_state! calls on_process_terminated(old_id) after it removes the old id. The callers report the new id with on_process_created after this, so a consumer sees the old id go before the new id comes.

Test

The PR adds the test item Process restart on env hash change reports the old process as terminated to test/test_process_reuse.jl. It runs one item twice with two env hashes and then shuts down the controller. It checks that on_process_terminated reports each id from on_process_created one time. Without the fix it fails, because the first id has no report. With the fix it passes. The other test items in test/test_process_reuse.jl also pass.

_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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant