* fix(ibus): restore engine after register_component teardown Stopping the Vocalinux IBus engine process destroys its register_component registration. IBus then runs check_global_engine(), which only searches register_engine_list and treats XML engines like xkb:es::spa as missing, clearing GlobalEngine even when Vocalinux was never selected. Capture a restorable engine before teardown and reselect it afterward so dead keys keep working after quit. Fixes #558 Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com> * fix(ibus): wait for engine exit before restoring GlobalEngine Adversarial review of #643 found that stop_engine_process() returned right after SIGTERM, so switch_engine() could race ahead of IBus check_global_engine() and still leave No global engine. Wait for the process to exit (SIGKILL fallback), retry engine restore, capture a shutdown fallback during prepare_engine, and re-apply XKB after the post-teardown engine switch. Addresses review of #558 / #643. Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com> * fix(ibus): prefer live engine over stale warmup on quit Bugbot on #643: stop() restored the warmup-cached engine even when the user had switched input source afterward, and inject_text refused to refresh that cache once set. Prefer the live non-Vocalinux engine at quit; fall back to the cache only when stuck on vocalinux/unavailable. Always refresh the shutdown fallback from inject_text. Also cover SIGKILL teardown escalation and restore-retry failure paths to lift patch coverage. Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com>
This commit is contained in:
Jatin K Malik
co-authored by
Jatin K Malik
Cursor Agent
Jatin K Malik
Cursor Agent
parent
45acb10d13
commit
7a5ddfde38
@@ -839,8 +839,13 @@ def start_engine_process() -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def stop_engine_process() -> None:
|
||||
"""Stop the IBus engine process if running."""
|
||||
def stop_engine_process(wait_timeout: float = 2.0) -> None:
|
||||
"""Stop the IBus engine process if running.
|
||||
|
||||
Waits for the process to actually exit so callers that restore the user's
|
||||
IBus engine afterward run after ``register_component`` teardown has been
|
||||
observed by ibus-daemon (issue #558).
|
||||
"""
|
||||
try:
|
||||
if not PID_FILE.exists():
|
||||
logger.debug("No PID file found, engine not running")
|
||||
@@ -857,6 +862,29 @@ def stop_engine_process() -> None:
|
||||
return
|
||||
|
||||
os.kill(pid, signal.SIGTERM)
|
||||
# Wait for exit: IBus clears/repairs GlobalEngine on component destroy,
|
||||
# which only runs after the process actually disconnects (#558).
|
||||
deadline = time.monotonic() + max(wait_timeout, 0.0)
|
||||
while time.monotonic() < deadline:
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except OSError:
|
||||
break
|
||||
time.sleep(0.05)
|
||||
else:
|
||||
# Still alive — escalate so teardown (and GlobalEngine repair) finishes.
|
||||
try:
|
||||
os.kill(pid, signal.SIGKILL)
|
||||
except OSError:
|
||||
pass
|
||||
kill_deadline = time.monotonic() + 0.5
|
||||
while time.monotonic() < kill_deadline:
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except OSError:
|
||||
break
|
||||
time.sleep(0.05)
|
||||
|
||||
logger.info(f"IBus engine process (PID {pid}) stopped")
|
||||
PID_FILE.unlink()
|
||||
except (OSError, ValueError, FileNotFoundError) as e:
|
||||
@@ -1206,7 +1234,18 @@ class IBusTextInjector:
|
||||
The Vocalinux engine is only needed while committing dictated text. Keeping
|
||||
the user's normal IBus/XKB engine active preserves dead-key composition and
|
||||
layout-specific behavior during ordinary typing.
|
||||
|
||||
Captures the current engine for shutdown restore only. Injection still
|
||||
resolves the restore target at commit time so a stale warmup value is not
|
||||
used mid-session (see #623); shutdown needs a fallback when quit leaves
|
||||
Vocalinux selected after a failed inject restore (#558).
|
||||
"""
|
||||
if not is_engine_active():
|
||||
captured = get_current_engine()
|
||||
if captured and captured != ENGINE_NAME:
|
||||
self._previous_engine = captured
|
||||
logger.debug(f"Captured current engine for shutdown restore: {captured}")
|
||||
|
||||
if not start_engine_process():
|
||||
raise IBusSetupError("Failed to start IBus engine process. Check logs for details.")
|
||||
|
||||
@@ -1303,18 +1342,49 @@ class IBusTextInjector:
|
||||
captured before injection (e.g. on GNOME/Wayland with no global IBus
|
||||
engine), restoration is skipped to avoid switching the user to a
|
||||
wrong layout (see issue #497).
|
||||
|
||||
Engine restoration must run *after* ``stop_engine_process()`` has
|
||||
observed process exit. Killing the process that called
|
||||
``register_component`` makes IBus run ``check_global_engine()``, which
|
||||
only searches ``register_engine_list`` and treats XML engines such as
|
||||
``xkb:es::spa`` as missing, clearing the global engine even when
|
||||
Vocalinux was never selected (issue #558).
|
||||
|
||||
Prefer the *live* engine at quit when it is a real non-Vocalinux
|
||||
engine, so a user who changed input source after warmup is not
|
||||
switched back to a stale cache. Fall back to ``_previous_engine`` only
|
||||
when the live engine is Vocalinux or unavailable (failed inject restore).
|
||||
"""
|
||||
if self._previous_engine:
|
||||
logger.info(f"Restoring previous engine: {self._previous_engine}")
|
||||
switch_engine(self._previous_engine)
|
||||
self._previous_engine = None
|
||||
current = get_current_engine()
|
||||
if current and current != ENGINE_NAME:
|
||||
restore_engine = current
|
||||
else:
|
||||
restore_engine = self._previous_engine
|
||||
if restore_engine == ENGINE_NAME:
|
||||
restore_engine = None
|
||||
|
||||
# Stop the engine process first and wait for exit — teardown can clear
|
||||
# IBus GlobalEngine (#558). Restore must land after that check.
|
||||
stop_engine_process()
|
||||
|
||||
if restore_engine:
|
||||
logger.info(f"Restoring previous engine after teardown: {restore_engine}")
|
||||
restored = False
|
||||
for attempt in range(3):
|
||||
if switch_engine(restore_engine):
|
||||
restored = True
|
||||
break
|
||||
time.sleep(0.15 * (attempt + 1))
|
||||
if not restored:
|
||||
logger.error(f"Failed to restore IBus engine after teardown: {restore_engine}")
|
||||
else:
|
||||
logger.info(
|
||||
"Skipping engine restoration — " "no valid engine was captured before injection"
|
||||
"Skipping engine restoration — no restorable engine was available at shutdown"
|
||||
)
|
||||
self._previous_engine = None
|
||||
|
||||
# Restore the XKB layout that was captured during setup
|
||||
# This ensures the user's original keyboard layout is preserved
|
||||
# XKB last: switching engines can override layout (#292). Re-apply after
|
||||
# the post-teardown engine restore so we don't lose the user's map.
|
||||
if self._previous_xkb_layout:
|
||||
layout, variant, option = self._previous_xkb_layout
|
||||
if layout:
|
||||
@@ -1322,9 +1392,6 @@ class IBusTextInjector:
|
||||
restore_xkb_layout(layout, variant, option)
|
||||
self._previous_xkb_layout = None
|
||||
|
||||
# Stop the engine process
|
||||
stop_engine_process()
|
||||
|
||||
def inject_text(self, text: str) -> bool:
|
||||
"""
|
||||
Inject text using the IBus engine.
|
||||
@@ -1352,6 +1419,9 @@ class IBusTextInjector:
|
||||
return False
|
||||
|
||||
restore_engine = current_engine
|
||||
# Always refresh the shutdown fallback to the live engine so a later
|
||||
# quit does not restore a stale warmup value (Bugbot on #643 / #558).
|
||||
self._previous_engine = current_engine
|
||||
logger.debug(
|
||||
f"Temporarily activating Vocalinux IBus engine (will restore {current_engine})"
|
||||
)
|
||||
|
||||
+176
-14
@@ -658,24 +658,85 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
with self.assertRaises(RuntimeError):
|
||||
IBusTextInjector()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
|
||||
def test_stop_restores_engine(self, mock_switch, mock_ensure_dir):
|
||||
"""Test stop() restores previous engine."""
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
def test_stop_restores_engine(
|
||||
self, mock_switch, mock_ensure_dir, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""Test stop() restores cached engine when live engine is unavailable (#558)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
parent = MagicMock()
|
||||
parent.attach_mock(mock_stop_proc, "stop_proc")
|
||||
parent.attach_mock(mock_switch, "switch")
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._previous_engine = "xkb:fr::fra"
|
||||
|
||||
injector.stop()
|
||||
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_called_once_with("xkb:fr::fra")
|
||||
self.assertEqual([c[0] for c in parent.mock_calls], ["stop_proc", "switch"])
|
||||
self.assertIsNone(injector._previous_engine)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:de::deu")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_no_previous_engine(self, mock_ensure_dir):
|
||||
def test_stop_prefers_live_engine_over_stale_warmup_cache(
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""Quit must restore the live input source, not a stale warmup cache (#643 Bugbot)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._previous_engine = "xkb:es::spa"
|
||||
|
||||
injector.stop()
|
||||
|
||||
mock_get_current.assert_called()
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_called_once_with("xkb:de::deu")
|
||||
self.assertIsNone(injector._previous_engine)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_restores_current_engine_when_previous_unset(
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""Warmup quit must restore the live engine after register_component teardown (#558)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
parent = MagicMock()
|
||||
parent.attach_mock(mock_stop_proc, "stop_proc")
|
||||
parent.attach_mock(mock_switch, "switch")
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
self.assertIsNone(injector._previous_engine)
|
||||
|
||||
injector.stop()
|
||||
|
||||
mock_get_current.assert_called()
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_called_once_with("xkb:es::spa")
|
||||
self.assertEqual([c[0] for c in parent.mock_calls], ["stop_proc", "switch"])
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_no_previous_engine(
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""Test stop() when no previous engine was saved."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
@@ -683,33 +744,49 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
|
||||
# Should not raise
|
||||
injector.stop()
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_not_called()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout")
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_restores_xkb_layout(
|
||||
self, mock_ensure_dir, mock_switch, mock_stop_proc, mock_restore_xkb
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_restore_xkb
|
||||
):
|
||||
"""Test stop() restores the captured XKB layout (#292)."""
|
||||
"""Test stop() restores XKB after post-teardown engine restore (#292/#558)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
parent = MagicMock()
|
||||
parent.attach_mock(mock_stop_proc, "stop_proc")
|
||||
parent.attach_mock(mock_switch, "switch")
|
||||
parent.attach_mock(mock_restore_xkb, "restore_xkb")
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._previous_engine = "xkb:fr::fra" # stale; live engine should win
|
||||
injector._previous_xkb_layout = ("es", "catalan", "compose:menu")
|
||||
|
||||
injector.stop()
|
||||
|
||||
mock_restore_xkb.assert_called_once_with("es", "catalan", "compose:menu")
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_called_once_with("xkb:es::spa")
|
||||
self.assertEqual(
|
||||
[c[0] for c in parent.mock_calls],
|
||||
["stop_proc", "switch", "restore_xkb"],
|
||||
)
|
||||
self.assertIsNone(injector._previous_xkb_layout)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout")
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_skips_xkb_restore_when_no_layout(
|
||||
self, mock_ensure_dir, mock_switch, mock_stop_proc, mock_restore_xkb
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_restore_xkb
|
||||
):
|
||||
"""Test stop() skips XKB restore when no layout was captured."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
@@ -720,6 +797,50 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
injector.stop()
|
||||
|
||||
mock_restore_xkb.assert_not_called()
|
||||
mock_switch.assert_not_called()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch(
|
||||
"vocalinux.text_injection.ibus_engine.get_current_engine",
|
||||
return_value="vocalinux",
|
||||
)
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_uses_cached_engine_when_stuck_on_vocalinux(
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""If quit finds vocalinux selected, fall back to the warmup-captured engine (#558)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._previous_engine = "xkb:es::spa"
|
||||
|
||||
injector.stop()
|
||||
|
||||
mock_get_current.assert_called()
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_called_once_with("xkb:es::spa")
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch(
|
||||
"vocalinux.text_injection.ibus_engine.get_current_engine",
|
||||
return_value="vocalinux",
|
||||
)
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_skips_restore_when_only_vocalinux_active(
|
||||
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""Do not try to reselect vocalinux after its process has been torn down."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector.stop()
|
||||
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_not_called()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_xkb_layout")
|
||||
@@ -757,6 +878,8 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
self.assertEqual(injector._previous_xkb_layout, ("fr", "azerty", ""))
|
||||
mock_restore_xkb.assert_called_once_with("fr", "azerty", "")
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:fr::fra")
|
||||
@patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("vocalinux.text_injection.ibus_engine.start_engine_process", return_value=True)
|
||||
@@ -768,6 +891,8 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
mock_wait_ready,
|
||||
mock_start_engine,
|
||||
mock_ensure_dir,
|
||||
mock_is_active,
|
||||
mock_get_current,
|
||||
):
|
||||
"""Test warmup starts the process without replacing the user's IBus engine."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
@@ -779,13 +904,13 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
mock_wait_ready.assert_called_once_with(require_active=False)
|
||||
mock_switch.assert_not_called()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:fr::fra")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
|
||||
@patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("vocalinux.text_injection.ibus_engine.start_engine_process", return_value=True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBusTextInjector._wait_for_engine_ready")
|
||||
def test_prepare_engine_does_not_cache_restore_engine(
|
||||
def test_prepare_engine_caches_shutdown_restore_engine(
|
||||
self,
|
||||
mock_wait_ready,
|
||||
mock_start_engine,
|
||||
@@ -793,15 +918,15 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
mock_is_active,
|
||||
mock_get_current,
|
||||
):
|
||||
"""Test warmup does not cache an engine that can become stale before injection."""
|
||||
"""Warmup caches the live engine so quit can restore after a stuck vocalinux (#558)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector.prepare_engine()
|
||||
|
||||
self.assertIsNone(injector._previous_engine)
|
||||
mock_get_current.assert_not_called()
|
||||
mock_is_active.assert_not_called()
|
||||
self.assertEqual(injector._previous_engine, "xkb:es::spa")
|
||||
mock_get_current.assert_called()
|
||||
mock_is_active.assert_called()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="libpinyin")
|
||||
@patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False)
|
||||
@@ -834,11 +959,48 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
|
||||
self.assertTrue(result)
|
||||
mock_sock.sendall.assert_called_once_with("Bonjour".encode("utf-8"))
|
||||
self.assertEqual(injector._previous_engine, "libpinyin")
|
||||
self.assertEqual(
|
||||
[call.args[0] for call in mock_switch.call_args_list],
|
||||
[ENGINE_NAME, "libpinyin"],
|
||||
)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:de::deu")
|
||||
@patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False)
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
@patch("socket.socket")
|
||||
@patch("vocalinux.text_injection.ibus_engine.SOCKET_PATH")
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_inject_text_refreshes_stale_shutdown_fallback(
|
||||
self,
|
||||
mock_ensure_dir,
|
||||
mock_socket_path,
|
||||
mock_socket_cls,
|
||||
mock_switch,
|
||||
mock_is_active,
|
||||
mock_get_current,
|
||||
):
|
||||
"""inject_text overwrites a stale warmup cache with the live engine (#643 Bugbot)."""
|
||||
from vocalinux.text_injection.ibus_engine import ENGINE_NAME, IBusTextInjector
|
||||
|
||||
mock_socket_path.exists.return_value = True
|
||||
mock_sock = MagicMock()
|
||||
mock_sock.__enter__.return_value = mock_sock
|
||||
mock_sock.__exit__.return_value = None
|
||||
mock_sock.recv.return_value = b"OK"
|
||||
mock_socket_cls.return_value = mock_sock
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._previous_engine = "xkb:es::spa"
|
||||
|
||||
self.assertTrue(injector.inject_text("Hallo"))
|
||||
self.assertEqual(injector._previous_engine, "xkb:de::deu")
|
||||
self.assertEqual(
|
||||
[call.args[0] for call in mock_switch.call_args_list],
|
||||
[ENGINE_NAME, "xkb:de::deu"],
|
||||
)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="libpinyin")
|
||||
@patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False)
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", side_effect=[True, False])
|
||||
|
||||
@@ -574,16 +574,20 @@ class TestIBusTextInjector(unittest.TestCase):
|
||||
self.assertIsNotNone(injector)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="other-engine")
|
||||
def test_ibus_text_injector_stop(self, mock_current_engine, mock_switch, mock_ensure_dir):
|
||||
"""Test IBusTextInjector stop restores previous engine."""
|
||||
def test_ibus_text_injector_stop(
|
||||
self, mock_current_engine, mock_switch, mock_stop_proc, mock_ensure_dir
|
||||
):
|
||||
"""Test IBusTextInjector stop restores previous engine after teardown."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._previous_engine = "other-engine"
|
||||
|
||||
injector.stop()
|
||||
mock_stop_proc.assert_called_once()
|
||||
mock_switch.assert_called_once_with("other-engine")
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
|
||||
@@ -4,6 +4,7 @@ Final comprehensive tests for IBus engine.
|
||||
These tests exercise code paths to improve coverage without platform-specific issues.
|
||||
"""
|
||||
|
||||
import signal
|
||||
import subprocess
|
||||
import sys
|
||||
import unittest
|
||||
@@ -319,6 +320,87 @@ class TestStopEngineProcess(unittest.TestCase):
|
||||
# Verify the function handles missing PID file gracefully
|
||||
self.assertIsNone(result)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.time.sleep", return_value=None)
|
||||
@patch("vocalinux.text_injection.ibus_engine.os.kill")
|
||||
@patch("vocalinux.text_injection.ibus_engine.Path")
|
||||
@patch("vocalinux.text_injection.ibus_engine.PID_FILE")
|
||||
def test_stop_engine_process_waits_for_exit(
|
||||
self, mock_pid_file, mock_path_cls, mock_kill, mock_sleep
|
||||
):
|
||||
"""SIGTERM path must wait until the process is gone before returning (#558)."""
|
||||
from vocalinux.text_injection.ibus_engine import stop_engine_process
|
||||
|
||||
mock_pid_file.exists.return_value = True
|
||||
mock_pid_file.read_text.return_value = "4242\n"
|
||||
|
||||
cmdline = MagicMock()
|
||||
cmdline.exists.return_value = True
|
||||
cmdline.read_text.return_value = "python /x/vocalinux/text_injection/ibus_engine.py --ibus"
|
||||
mock_path_cls.return_value = cmdline
|
||||
|
||||
# SIGTERM succeeds; first liveness check still alive; second gone.
|
||||
mock_kill.side_effect = [None, None, OSError(3, "No such process")]
|
||||
|
||||
stop_engine_process(wait_timeout=1.0)
|
||||
|
||||
self.assertEqual(mock_kill.call_args_list[0][0], (4242, signal.SIGTERM))
|
||||
self.assertGreaterEqual(mock_kill.call_count, 3)
|
||||
mock_pid_file.unlink.assert_called()
|
||||
mock_sleep.assert_called()
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.time.sleep", return_value=None)
|
||||
@patch("vocalinux.text_injection.ibus_engine.os.kill")
|
||||
@patch("vocalinux.text_injection.ibus_engine.Path")
|
||||
@patch("vocalinux.text_injection.ibus_engine.PID_FILE")
|
||||
def test_stop_engine_process_sigkills_when_term_ignored(
|
||||
self, mock_pid_file, mock_path_cls, mock_kill, mock_sleep
|
||||
):
|
||||
"""Escalate to SIGKILL if the engine ignores SIGTERM (#558 wait path)."""
|
||||
from vocalinux.text_injection.ibus_engine import stop_engine_process
|
||||
|
||||
mock_pid_file.exists.return_value = True
|
||||
mock_pid_file.read_text.return_value = "4242\n"
|
||||
|
||||
cmdline = MagicMock()
|
||||
cmdline.exists.return_value = True
|
||||
cmdline.read_text.return_value = "python /x/vocalinux/text_injection/ibus_engine.py --ibus"
|
||||
mock_path_cls.return_value = cmdline
|
||||
|
||||
# SIGTERM, then keep reporting alive until wait timeout, then SIGKILL succeeds.
|
||||
mock_kill.side_effect = [None, None, None, OSError(3, "No such process")]
|
||||
with patch(
|
||||
"vocalinux.text_injection.ibus_engine.time.monotonic",
|
||||
side_effect=[0.0, 0.0, 2.1, 2.1, 2.2, 2.7],
|
||||
):
|
||||
stop_engine_process(wait_timeout=2.0)
|
||||
|
||||
self.assertEqual(mock_kill.call_args_list[0][0], (4242, signal.SIGTERM))
|
||||
self.assertTrue(any(c[0] == (4242, signal.SIGKILL) for c in mock_kill.call_args_list))
|
||||
mock_pid_file.unlink.assert_called()
|
||||
|
||||
|
||||
class TestStopRestoreRetry(unittest.TestCase):
|
||||
"""Coverage for stop() restore retry after teardown."""
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
|
||||
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=False)
|
||||
@patch("vocalinux.text_injection.ibus_engine.time.sleep", return_value=None)
|
||||
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
def test_stop_retries_failed_engine_restore(
|
||||
self, mock_ensure_dir, mock_sleep, mock_switch, mock_get_current, mock_stop_proc
|
||||
):
|
||||
"""stop() retries switch_engine after teardown when the first attempts fail."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector.stop()
|
||||
|
||||
self.assertEqual(mock_switch.call_count, 3)
|
||||
mock_switch.assert_called_with("xkb:es::spa")
|
||||
self.assertEqual(mock_sleep.call_count, 3)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user