From 1827667cb26742dbbd557d19be081960d5d59c3a Mon Sep 17 00:00:00 2001 From: Tanmoy Sarkar Date: Tue, 9 Jun 2026 23:08:29 +0530 Subject: [PATCH] test(companion): Assert HTTP worker path is unchanged Add two arbiter regression tests. A worker exit is still reaped normally (tmp closed, child_exit called) while a companion manager pid is registered, so the companion reap branch does not swallow worker exits. And an HTTP worker is still spawned and recorded as before when companions are configured, so the companion config never touches the worker path. Co-Authored-By: Claude Opus 4.8 --- docs/design/companion-process-manager.md | 2 +- tests/test_arbiter.py | 31 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/design/companion-process-manager.md b/docs/design/companion-process-manager.md index f26ba0b2..2fb501e3 100644 --- a/docs/design/companion-process-manager.md +++ b/docs/design/companion-process-manager.md @@ -699,7 +699,7 @@ No per-companion logic in Arbiter. - [x] Add tests for state transitions. - [x] Add tests for control commands. - [x] Add tests for transactional reread. -- [ ] Add tests that HTTP worker behavior is unchanged. +- [x] Add tests that HTTP worker behavior is unchanged. ## 21. Test Plan diff --git a/tests/test_arbiter.py b/tests/test_arbiter.py index 523bbec5..9f50018c 100644 --- a/tests/test_arbiter.py +++ b/tests/test_arbiter.py @@ -205,6 +205,37 @@ def test_stop_companion_manager_clears_pid_when_already_gone(): assert arbiter.companion_manager_pid == 0 +@mock.patch('os.waitpid') +def test_worker_reap_unaffected_by_companion_manager(mock_os_waitpid): + # A worker exit is still reaped normally while a companion manager runs; + # the companion reap branch must not swallow worker exits. + mock_os_waitpid.side_effect = [(42, 0), (0, 0)] + arbiter = gunicorn.arbiter.Arbiter(DummyApplication()) + arbiter.cfg.settings['child_exit'] = mock.Mock() + arbiter.companion_manager_pid = 9999 + mock_worker = mock.Mock() + arbiter.WORKERS = {42: mock_worker} + arbiter.reap_workers() + mock_worker.tmp.close.assert_called_with() + arbiter.cfg.child_exit.assert_called_with(arbiter, mock_worker) + assert arbiter.companion_manager_pid == 9999 + + +@mock.patch('os.fork', return_value=77) +def test_spawn_worker_unaffected_by_companions(mock_os_fork): + # With companions configured, an HTTP worker is still spawned and recorded + # exactly as before; companion config does not touch the worker path. + arbiter = gunicorn.arbiter.Arbiter(DummyApplication()) + arbiter.cfg.set("companion_workers", [{"name": "rq", "target": "pkg:run"}]) + arbiter.pid = 1234 + arbiter.WORKERS = {} # instance dict, do not mutate the shared class attr + mock_worker = mock.Mock() + arbiter.worker_class = mock.Mock(return_value=mock_worker) + pid = arbiter.spawn_worker() + assert pid == 77 + assert arbiter.WORKERS[77] is mock_worker + + def test_close_gunicorn_fds_in_manager_child(): arbiter = gunicorn.arbiter.Arbiter(DummyApplication()) listener = mock.Mock()