test(gateway-service): compute expected TimeoutStopSec from SSOT, not literal
The literal "TimeoutStopSec=90" assertion was correct when default restart_drain_timeout was 60s (max(60,60)+30=90). The default was bumped to 180s in hermes_cli/config.py, making the unit emit TimeoutStopSec=210 — still satisfying issue #8202 ("timeout must exceed drain so systemd doesn't SIGKILL pre-cleanup") but breaking the literal assertion. Refactor: compute the expected minimum from gateway.restart.DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT and assert >=. The test now expresses the invariant directly and survives future drain bumps. Mutation-tested mentally: deleting the +30 headroom in hermes_cli/gateway.py:1636 still fails this test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7672bd9b4f
commit
ed0957256b
@ -5,6 +5,8 @@ import pwd
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import re
|
||||
|
||||
import pytest
|
||||
|
||||
import hermes_cli.gateway as gateway_cli
|
||||
@ -107,6 +109,18 @@ class TestSystemdServiceRefresh:
|
||||
]
|
||||
|
||||
|
||||
def _expected_min_timeout_stop_sec() -> int:
|
||||
"""Compute the minimum TimeoutStopSec that systemd unit generation
|
||||
should emit, mirroring hermes_cli/gateway.py:1636. The unit must give
|
||||
the gateway at least drain_timeout + 30s of headroom so post-interrupt
|
||||
cleanup (tool subprocess kill, adapter disconnect) finishes before
|
||||
systemd escalates to SIGKILL on the cgroup (issue #8202).
|
||||
"""
|
||||
from gateway.restart import DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT
|
||||
drain = int(DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT or 0)
|
||||
return max(60, drain) + 30
|
||||
|
||||
|
||||
class TestGeneratedSystemdUnits:
|
||||
def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self):
|
||||
unit = gateway_cli.generate_systemd_unit(system=False)
|
||||
@ -115,10 +129,17 @@ class TestGeneratedSystemdUnits:
|
||||
assert "ExecStop=" not in unit
|
||||
assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit
|
||||
assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit
|
||||
# TimeoutStopSec must exceed the default drain_timeout (60s) so
|
||||
# TimeoutStopSec must exceed the configured drain_timeout so
|
||||
# systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
|
||||
# (tool subprocess kill, adapter disconnect) runs — issue #8202.
|
||||
assert "TimeoutStopSec=90" in unit
|
||||
# Compute the expected minimum from the SSOT default rather than
|
||||
# hard-coding a value that drifts with config bumps.
|
||||
expected = _expected_min_timeout_stop_sec()
|
||||
match = re.search(r"^TimeoutStopSec=(\d+)", unit, flags=re.MULTILINE)
|
||||
assert match is not None, "TimeoutStopSec directive missing"
|
||||
assert int(match.group(1)) >= expected, (
|
||||
f"TimeoutStopSec={match.group(1)} < expected {expected}"
|
||||
)
|
||||
|
||||
def test_user_unit_includes_resolved_node_directory_in_path(self, monkeypatch):
|
||||
monkeypatch.setattr(gateway_cli.shutil, "which", lambda cmd: "/home/test/.nvm/versions/node/v24.14.0/bin/node" if cmd == "node" else None)
|
||||
@ -134,10 +155,17 @@ class TestGeneratedSystemdUnits:
|
||||
assert "ExecStop=" not in unit
|
||||
assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit
|
||||
assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit
|
||||
# TimeoutStopSec must exceed the default drain_timeout (60s) so
|
||||
# TimeoutStopSec must exceed the configured drain_timeout so
|
||||
# systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
|
||||
# (tool subprocess kill, adapter disconnect) runs — issue #8202.
|
||||
assert "TimeoutStopSec=90" in unit
|
||||
# Compute the expected minimum from the SSOT default rather than
|
||||
# hard-coding a value that drifts with config bumps.
|
||||
expected = _expected_min_timeout_stop_sec()
|
||||
match = re.search(r"^TimeoutStopSec=(\d+)", unit, flags=re.MULTILINE)
|
||||
assert match is not None, "TimeoutStopSec directive missing"
|
||||
assert int(match.group(1)) >= expected, (
|
||||
f"TimeoutStopSec={match.group(1)} < expected {expected}"
|
||||
)
|
||||
assert "WantedBy=multi-user.target" in unit
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user