From 1fc57b22a8a1e23fdc9d2f2a9074981a4e3949a9 Mon Sep 17 00:00:00 2001 From: Tanmoy Sarkar Date: Tue, 9 Jun 2026 23:06:28 +0530 Subject: [PATCH] test(companion): Add transactional reread test Add the missing transactionality assertion to the existing reread coverage: a batch that would change one companion and add another but also contains a duplicate name is rejected as a whole. The process set and configs stay untouched and no fork or kill happens, proving nothing is applied on validation failure. Co-Authored-By: Claude Opus 4.8 --- docs/design/companion-process-manager.md | 2 +- tests/test_companion_manager.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/design/companion-process-manager.md b/docs/design/companion-process-manager.md index 6b60b3e7..f26ba0b2 100644 --- a/docs/design/companion-process-manager.md +++ b/docs/design/companion-process-manager.md @@ -698,7 +698,7 @@ No per-companion logic in Arbiter. - [x] Add tests for config validation. - [x] Add tests for state transitions. - [x] Add tests for control commands. -- [ ] Add tests for transactional reread. +- [x] Add tests for transactional reread. - [ ] Add tests that HTTP worker behavior is unchanged. ## 21. Test Plan diff --git a/tests/test_companion_manager.py b/tests/test_companion_manager.py index 3016572b..3fe6e48b 100644 --- a/tests/test_companion_manager.py +++ b/tests/test_companion_manager.py @@ -285,6 +285,23 @@ def test_reread_duplicate_name_keeps_old(): assert "duplicate" in result["error"] +def test_reread_validation_failure_mutates_nothing(): + manager = make_manager("rq") + original_config = manager.processes["rq"].config + original_names = set(manager.processes) + # This batch would change "rq" and add "scheduler", but the duplicate + # "scheduler" makes the whole reread invalid: nothing must be applied. + bad = [make_config("rq", env={"X": "1"}), make_config("scheduler"), + make_config("scheduler")] + with mock.patch("os.fork") as fork, mock.patch("os.kill") as kill: + result = manager.reread_config(bad) + assert result["ok"] is False and result["kept_old_config"] is True + assert set(manager.processes) == original_names + assert manager.processes["rq"].config is original_config + fork.assert_not_called() + kill.assert_not_called() + + def test_handle_command_reread_no_loader(): manager = make_manager("rq") with pytest.raises(CommandError):