feat: process isolation and fork-safe test runners - #566
Conversation
24eb3fa to
a6de914
Compare
|
Thanks for the PR ❤️ I'll try to look a bit more at it today or tomorrow. From a high level, I think we get the following (?):
I think the hot-fork + mutate_only_covered_lines combination is mabye not as expected: We run pytest via What this PR would fix:
What this PR nearly fixes:
What this PR does not attempt to fix:
|
Otto-AA
left a comment
There was a problem hiding this comment.
I won't have time to review the code in the next weeks, so here are some high level inputs (feel free to incorporate or to merge without changes):
- the
process_isolationandhot_fork_warmupshould be explained in the README. As a user, I'd like to know when to use which of those. - I feel like the naming
forkandhot-forkdoes not reflect the differences between those two. I'm not sure how to better name them though - I feel like it could fix #528, but currently does not. I haven't verified it though. (could also be fixed in a separate PR)
|
|
||
|
|
||
| def test_hot_fork_runs_end_to_end(tmp_path: Path): | ||
| project = tmp_path / "hot_fork_basic" |
There was a problem hiding this comment.
Is there a reason you use a different setup than for the other tests?
This seems to work:
def test_hot_fork_runs_end_to_end():
assert run_mutmut_on_project("hot_fork_basic") == snapshot(
{
"mutants/src/hf_calc/__init__.py.meta": {
"hf_calc.x_add__mutmut_1": 1,
"hf_calc.x_sub__mutmut_1": 1,
"hf_calc.x_mul__mutmut_1": 1,
"hf_calc.x_untested__mutmut_1": 33,
"hf_calc.x_untested__mutmut_2": 33,
}
}
)There was a problem hiding this comment.
I believe this branch predated adding the snapshot to the project and I must have just gone blind to it. nice catch!
good call on these suggestions, especially 528. I'll see if I can slot these in neatly. I agree hot-fork is not the best name, I named it based on the idea that the fork gets warmed, but I should find something better. I'll see what claude can come up with, definitely has better ideas than I can come up with for naming things. |
Add pipe-based fork isolation helpers (run_in_fork_with_result, run_in_fork) that run functions in forked children so the parent process never imports pytest/conftest and stays fork-safe. This is the foundation for the upcoming hot-fork runner where the parent acts purely as an orchestrator. Also add OrchestratorCrashError, which reports the crashed orchestrator's exit code, the lost in-flight mutants (truncated past 10), an optional crash-log path, and instructions to resume with 'mutmut run'.
Introduce a MutantRunner ABC that owns the process-isolation strategy for testing mutants and also fronts the surrounding test operations (stats collection, clean tests, forced-fail, test listing), so __main__ no longer drives os.fork() directly. ForkRunner encapsulates the traditional os.fork()-per-mutant loop that lived inline in _run(): submit() forks a child under a CPU/wall timeout, and wait_for_result() reaps one child into a MutantResult. get_mutant_runner() selects the runner from the new process_isolation config (ProcessIsolation enum; only 'fork' is wired up here, 'hot-fork' raises pending a later commit). _run() now drives the runner through submit/has_capacity/wait_for_result/ pending_count/shutdown and registers results by mutant name. The stale-stats protections are preserved verbatim: _check_test_to_mutant_associations() still runs, and collect_or_load_stats() keeps its apply_config_invalidation path (now routed through MutantRunner.collect_stats/list_all_tests). Behavior for the default fork path is unchanged; the full suite (incl. the e2e run) is green.
Add a single-orchestrator process-isolation strategy for projects whose test
setup makes the parent fork-unsafe (gevent monkey-patching, grpc, torch):
parent (clean) -> orchestrator (imports pytest once) -> N grandchildren
The parent never imports pytest/conftest. The orchestrator imports pytest a
single time, warms up (configurable via hot_fork_warmup: collect/import/none),
then forks one grandchild per mutant, streaming results back over a pipe and
reaping via a SIGCHLD self-pipe. If the orchestrator crashes, in-flight mutants
are re-submitted to a fresh orchestrator up to max_orchestrator_restarts times
before raising OrchestratorCrashError. Stats/clean-test/forced-fail/test-listing
all run in short-lived forks (StatsResult carries the collected mapping back to
the parent), so the parent stays clean throughout.
Selected via process_isolation = "hot-fork"; get_mutant_runner() now builds it.
Supporting pieces: HotForkWarmup config + validation, TestRunner.warm_up(),
models/results.StatsResult, and utils/logging_utils for the orchestrator's
file-only logging and crash logs.
Validated end-to-end: on the my_lib project, hot-fork produces byte-identical
verdicts to fork (112 mutants, same 37/64/10/1 distribution). Adds a subprocess
e2e smoke test (hot_fork_basic) plus factory/config-validation unit tests.
d93b852 to
d18d722
Compare
The coverage pre-run used to import the test suite into the main process and then evict every module it had loaded, so the stats run imported them a second time. gather_coverage now runs in a throwaway fork and sends back only the measured line numbers. Also: - run_in_fork_with_result carries the child's traceback across the pipe - e2e regression guard: a dependency that refuses a second init per process
Pluggable process isolation, with a fork-safe
hot-forkrunnerAdds a
MutantRunnerabstraction that owns the process-isolation strategy for mutation testingTwo strategies:
fork(default) the existing fork-per-mutant behaviour, extracted intoForkRunner.No behavioural change.
hot-forkparent (clean) -> orchestrator (imports pytest once) -> N grandchildren,for projects whose test setup makes the parent fork-unsafe (gevent monkey-patching,
grpc, torch). The parent never imports pytest or conftest; the orchestrator imports it
exactly once and forks a grandchild per mutant. Results stream back over a
length-prefixed pipe protocol, and grandchildren are reaped via a SIGCHLD self-pipe.
A crashed orchestrator is restarted with its in-flight mutants re-submitted, up to
max_orchestrator_restarts, before raisingOrchestratorCrashError.Stats collection, clean tests, forced-fail and test listing all run in short-lived forks
(
run_in_fork/run_in_fork_with_result) so the parent stays clean throughout.Config
process_isolationforkforkorhot-forkhot_fork_warmupcollectcollect/import/nonemax_orchestrator_restarts3OrchestratorCrashErrorpreload_modules_fileNonelog_to_fileFalselog_file_pathmutants/mutmut-debug.logIncidental cleanups
Breaks the
mutation/trampoline.py->__main__import cycle:MutmutProgrammaticFailExceptionmoves to
core.pyandrecord_trampoline_hittostats.py, which removes the_set_mutant_under_testlate-import shim.src/now has zerofrom mutmut.__main__imports. Also drops the dead pid-keyed bookkeeping from
SourceFileMutationData(
register_pid/register_result/stop_children), superseded byMutantResult, andsets
platform = linuxinmypy.ini, mutmut is (currently) POSIX-only, so type-checkingelsewhere otherwise flags
os.fork/resource.setrlimit/signal.SIGXCPUas missing.Testing
Verified that
forkandhot-forkproduce identical verdicts on themy_libe2e project.New coverage for the pipe protocol (including payloads larger than the pipe buffer),
ForkRunner/HotForkRunnerno-test-mutant handling,get_mutant_runnerdispatch,OrchestratorCrashErrormessaging, and config validation, plus a subprocess e2e smoketest (
tests/e2e/test_e2e_hot_fork.pywith thehot_fork_basicproject) exercising thefull hot-fork path.