From 92a005293a99590ecc945c7bd32285f75be945b2 Mon Sep 17 00:00:00 2001 From: brightonanc Date: Thu, 5 May 2022 14:16:54 -0400 Subject: [PATCH] Cleanup and a few new tests to appease the formatting, branch coverage, etc. Merges needed were: * qutebrowser/keyinput/basekeyparser.py for the new count precaution (commit 51aa7ab) * tests/unit/keyinput/test_basekeyparser.py for new count precaution (commit 51aa7ab) unit test --- qutebrowser/browser/browsertab.py | 4 +- qutebrowser/keyinput/basekeyparser.py | 8 +- qutebrowser/keyinput/keyutils.py | 2 +- qutebrowser/keyinput/modeman.py | 13 ++-- qutebrowser/keyinput/modeparsers.py | 8 +- tests/unit/keyinput/test_basekeyparser.py | 28 ++++++- tests/unit/keyinput/test_modeman.py | 94 +++++++++++++++++++++-- tests/unit/keyinput/test_modeparsers.py | 6 +- 8 files changed, 137 insertions(+), 26 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index c4d912705..9884dc472 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -1122,7 +1122,7 @@ class AbstractTab(QWidget): evt.posted = True # type: ignore[attr-defined] QApplication.postEvent(recipient, evt) - def send_event(self, evt: QEvent) -> bool: + def send_event(self, evt: QEvent) -> Optional[bool]: """Send the given event to the underlying widget. The event will be sent via QApplication.sendEvent. @@ -1139,7 +1139,7 @@ class AbstractTab(QWidget): if recipient is None: # https://github.com/qutebrowser/qutebrowser/issues/3888 log.webview.warning("Unable to find event target!") - return + return None evt.posted = True # type: ignore[attr-defined] return QApplication.sendEvent(recipient, evt) diff --git a/qutebrowser/keyinput/basekeyparser.py b/qutebrowser/keyinput/basekeyparser.py index 55e430c51..2da5c3180 100644 --- a/qutebrowser/keyinput/basekeyparser.py +++ b/qutebrowser/keyinput/basekeyparser.py @@ -8,7 +8,7 @@ import string import types import dataclasses import traceback -from typing import Optional +from typing import Optional, List from collections.abc import Mapping, MutableMapping, Sequence from qutebrowser.qt.core import QObject, pyqtSignal, pyqtSlot @@ -199,7 +199,7 @@ class BaseKeyParser(QObject): self._pure_sequence = keyutils.KeySequence() self._sequence = keyutils.KeySequence() self._count = '' - self._count_keyposs: Sequence[int] = [] + self._count_keyposs: List[int] = [] self._mode = mode self._do_log = do_log self.passthrough = passthrough @@ -411,7 +411,7 @@ class BaseKeyParser(QObject): self._debug_log("Definitive match for '{}'.".format( result.sequence)) try: - count = int(self._count) if self._count else None + count_int = int(self._count) if self._count else None flag_do_execute = True except ValueError as err: message.error(f"Failed to parse count: {err}", @@ -420,7 +420,7 @@ class BaseKeyParser(QObject): self.clear_partial_keys.emit() self.clear_keystring() if flag_do_execute: - self.execute(result.command, count) + self.execute(result.command, count_int) elif result.match_type == QKeySequence.SequenceMatch.PartialMatch: self._debug_log("No match for '{}' (added {})".format( result.sequence, info)) diff --git a/qutebrowser/keyinput/keyutils.py b/qutebrowser/keyinput/keyutils.py index f8e7b6f23..9fd94daec 100644 --- a/qutebrowser/keyinput/keyutils.py +++ b/qutebrowser/keyinput/keyutils.py @@ -545,7 +545,7 @@ class QueuedKeyEventPair: key_event: KeyEvent key_info_press: KeyInfo - key_info_release: Union[KeyInfo, None] + key_info_release: Optional[KeyInfo] @classmethod def from_event_press(cls, event: QKeyEvent) -> 'QueuedKeyEventPair': diff --git a/qutebrowser/keyinput/modeman.py b/qutebrowser/keyinput/modeman.py index c21cf37f3..a2d82d613 100644 --- a/qutebrowser/keyinput/modeman.py +++ b/qutebrowser/keyinput/modeman.py @@ -9,7 +9,7 @@ from PyQt5.QtWidgets import QApplication import functools import dataclasses from typing import Union, cast -from collections.abc import Mapping, MutableMapping, Callable, Sequence +from collections.abc import Mapping, MutableMapping, Callable, List from qutebrowser.qt import machinery from qutebrowser.qt.core import pyqtSlot, pyqtSignal, Qt, QObject, QEvent @@ -119,7 +119,7 @@ def init(win_id: int, parent: QObject) -> 'ModeManager': # infeasible as 'prompt-container' is registered as command-only. # Plus, I imagine the use case for such a thing is quite rare. allow_forward=False, - forward_widget_name=None, #'prompt-container' + forward_widget_name=None, # 'prompt-container' allow_partial_timeout=False), usertypes.KeyMode.yesno: @@ -257,7 +257,7 @@ class ModeManager(QObject): self._releaseevents_to_pass: set[keyutils.KeyEvent] = set() # Set after __init__ self.hintmanager = cast(hints.HintManager, None) - self._partial_match_events: Sequence[keyutils.QueuedKeyEventPair] = [] + self._partial_match_events: List[keyutils.QueuedKeyEventPair] = [] self.forward_partial_key.connect(self.forward_partial_match_event) self._partial_timer = usertypes.Timer(self, 'partial-match') self._partial_timer.setSingleShot(True) @@ -297,7 +297,7 @@ class ModeManager(QObject): # must manually forward the events self.forward_all_partial_match_events(self.mode, stop_timer=True) - match = QKeySequence.NoMatch + match = QKeySequence.SequenceMatch.NoMatch else: match = parser.handle(event, dry_run=dry_run) @@ -402,7 +402,7 @@ class ModeManager(QObject): @pyqtSlot(usertypes.KeyMode, str) def forward_partial_match_event(self, mode: usertypes.KeyMode, text: str = None) -> None: - """Forward the oldest partial match event for a given mode + """Forward the oldest partial match event for a given mode. Args: mode: The mode from which the forwarded match is. @@ -454,7 +454,7 @@ class ModeManager(QObject): @pyqtSlot(usertypes.KeyMode) def forward_all_partial_match_events(self, mode: usertypes.KeyMode, *, stop_timer: bool = False) -> None: - """Forward all partial match events for a given mode + """Forward all partial match events for a given mode. Args: mode: The mode from which the forwarded match is. @@ -615,6 +615,7 @@ class ModeManager(QObject): self.leave(self.mode, 'leave current') def change_mode(self, mode: usertypes.KeyMode) -> None: + """Changes mode and forwards partial match events if present.""" # catches the case where change of mode is not keys, e.g. mouse click self.forward_all_partial_match_events(self.mode, stop_timer=True) self.mode = mode diff --git a/qutebrowser/keyinput/modeparsers.py b/qutebrowser/keyinput/modeparsers.py index e2e3e2cbf..b253fbc84 100644 --- a/qutebrowser/keyinput/modeparsers.py +++ b/qutebrowser/keyinput/modeparsers.py @@ -10,7 +10,7 @@ Module attributes: import traceback import enum -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, List from collections.abc import Sequence from qutebrowser.qt.core import pyqtSlot, Qt, QObject @@ -146,7 +146,7 @@ class HintKeyParser(basekeyparser.BaseKeyParser): self._hintmanager = hintmanager self._filtertext = '' self._last_press = LastPress.none - self._partial_match_events: Sequence[keyutils.QueuedKeyEventPair] = [] + self._partial_match_events: List[keyutils.QueuedKeyEventPair] = [] self.keystring_updated.connect(self._hintmanager.handle_partial_key) self._command_parser.forward_partial_key.connect( self.forward_partial_match_event) @@ -253,7 +253,7 @@ class HintKeyParser(basekeyparser.BaseKeyParser): @pyqtSlot(str) def forward_partial_match_event(self, text: str = None) -> None: - """Forward the oldest partial match event + """Forward the oldest partial match event. Args: text: The expected text to be forwarded. Only used for debug @@ -277,7 +277,7 @@ class HintKeyParser(basekeyparser.BaseKeyParser): @pyqtSlot() def forward_all_partial_match_events(self, *, stop_timer: bool = False) -> None: - """Forward all partial match events + """Forward all partial match events. Args: stop_timer: If true, stop the partial timer as well. Default is False. diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 0f59d9a37..eaf91a0ed 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -112,7 +112,7 @@ def test_mixed_count(keyparser, config_stub, input_key, final_count, final_match elif result == QKeySequence.SequenceMatch.PartialMatch: assert keyparser._count == str(final_count) else: - assert False, 'Not Implemented' + pytest.fail('Not Implemented') def test_empty_binding(keyparser, config_stub): @@ -352,8 +352,13 @@ class TestHandle: keyparser.execute.assert_not_called() seq = list(seq) + [Qt.Key.Key_Z] signals = [keyparser.forward_partial_key] * len(seq) + info = keyutils.KeyInfo(seq[-1], Qt.KeyboardModifier.NoModifier) + result = keyparser.handle(info.to_event(), dry_run=True) + assert result == QKeySequence.SequenceMatch.NoMatch with qtbot.wait_signals(signals) as blocker: - handle_text(keyparser, seq[-1]) + result = keyparser.handle(info.to_event()) + assert result == QKeySequence.SequenceMatch.NoMatch + assert blocker.signal_triggered assert forward_partial_key.call_args_list == [ ((str(keyutils.KeyInfo(key, Qt.KeyboardModifier.NoModifier)),),) for key in seq ] @@ -402,6 +407,25 @@ class TestHandle: ) assert keyparser._sequence == keyseq('f') + def test_expansive_mapping(self, config_stub, keyparser): + config_stub.val.bindings.key_mappings = { + 'x': 'ab' + } + config_stub.val.bindings.commands = { + 'normal': { + 'abc': 'message-info abc' + } + } + + result = keyparser.handle( + keyutils.KeyInfo(Qt.Key_X, Qt.KeyboardModifier.NoModifier).to_event()) + assert result == QKeySequence.SequenceMatch.PartialMatch + result = keyparser.handle( + keyutils.KeyInfo(Qt.Key_2, Qt.KeyboardModifier.NoModifier).to_event()) + # Check that count is not evaluated when an expansive mapping occurs. + # This behavior may change in the future. + assert result == QKeySequence.SequenceMatch.NoMatch + class TestCount: diff --git a/tests/unit/keyinput/test_modeman.py b/tests/unit/keyinput/test_modeman.py index f6583c2ce..c8a080896 100644 --- a/tests/unit/keyinput/test_modeman.py +++ b/tests/unit/keyinput/test_modeman.py @@ -4,11 +4,11 @@ import pytest -from qutebrowser.qt.core import Qt, QObject, pyqtSignal, QTimer +from qutebrowser.qt.core import Qt, QObject, pyqtSignal, QTimer, QEvent from qutebrowser.qt.gui import QKeyEvent, QKeySequence -from qutebrowser.utils import usertypes -from qutebrowser.keyinput import keyutils +from qutebrowser.utils import usertypes, objreg +from qutebrowser.keyinput import keyutils, basekeyparser from qutebrowser.misc import objects @@ -118,6 +118,7 @@ def test_partial_keychain_timeout(modeman_with_timeout, config_stub, qtbot, data parser = modeman_with_timeout.parsers[mode] assert not timer.isActive() + behavior = None for key, behavior in data_sequence: keyinfo = keyutils.KeyInfo(key, Qt.KeyboardModifier.NoModifier) if behavior == 'timer_active': @@ -144,8 +145,7 @@ def test_partial_keychain_timeout(modeman_with_timeout, config_stub, qtbot, data assert (timeout - (timeout//4)) < timer.remainingTime() assert timer.isActive() else: - # Unreachable - assert False + pytest.fail('Unreachable') if behavior in ['timer_active', 'timer_reset']: # Now simulate a timeout and check the keystring has been cleared. with qtbot.wait_signal(modeman_with_timeout.keystring_updated) as blocker: @@ -154,3 +154,87 @@ def test_partial_keychain_timeout(modeman_with_timeout, config_stub, qtbot, data parser.fake_clear_keystring_called = False assert blocker.args == [mode, ''] + +class FakeEventFilter(QObject): + def eventFilter(self, obj: QObject, event: QEvent) -> bool: + return True + + +@pytest.fixture +def modeman_with_basekeyparser(mode_manager, config_stub): + fake_event_filter = FakeEventFilter() + objreg.register('fake-event-filter', fake_event_filter, scope='window', + window=0) + config_stub.val.bindings.default = {} + config_stub.val.bindings.commands = { + 'normal': { + 'bb': 'message-info bb', + 'byy': 'message-info byy', + } + } + config_stub.val.bindings.key_mappings = {} + mode = usertypes.KeyMode.normal + mode_manager.register(mode, + basekeyparser.BaseKeyParser(mode=mode, + win_id=0, + passthrough=True, + forward_widget_name='fake-event-filter')) + yield mode_manager + objreg.delete('fake-event-filter', scope='window', window=0) + + +def test_release_forwarding(modeman_with_basekeyparser): + mwb = modeman_with_basekeyparser + + info_b = keyutils.KeyInfo(Qt.Key.Key_B, Qt.KeyboardModifier.NoModifier) + info_c = keyutils.KeyInfo(Qt.Key.Key_C, Qt.KeyboardModifier.NoModifier) + + res = mwb.handle_event(info_b.to_event(QEvent.KeyPress)) + assert res + assert 1 == len(mwb._partial_match_events) + assert not mwb._partial_match_events[0].is_released() + assert 0 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_c.to_event(QEvent.KeyPress)) + assert res + assert 0 == len(mwb._partial_match_events) + assert 2 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_b.to_event(QEvent.KeyRelease)) + assert not res + assert 0 == len(mwb._partial_match_events) + assert 1 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_c.to_event(QEvent.KeyRelease)) + assert not res + assert 0 == len(mwb._partial_match_events) + assert 0 == len(mwb._releaseevents_to_pass) + + info_y = keyutils.KeyInfo(Qt.Key.Key_Y, Qt.KeyboardModifier.NoModifier) + + res = mwb.handle_event(info_b.to_event(QEvent.KeyPress)) + assert res + assert 1 == len(mwb._partial_match_events) + assert not mwb._partial_match_events[0].is_released() + assert 0 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_y.to_event(QEvent.KeyPress)) + assert res + assert 2 == len(mwb._partial_match_events) + assert not mwb._partial_match_events[0].is_released() + assert not mwb._partial_match_events[1].is_released() + assert 0 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_y.to_event(QEvent.KeyRelease)) + assert res + assert 2 == len(mwb._partial_match_events) + assert not mwb._partial_match_events[0].is_released() + assert mwb._partial_match_events[1].is_released() + assert 0 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_c.to_event(QEvent.KeyPress)) + assert res + assert 0 == len(mwb._partial_match_events) + assert 2 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_c.to_event(QEvent.KeyRelease)) + assert not res + assert 0 == len(mwb._partial_match_events) + assert 1 == len(mwb._releaseevents_to_pass) + res = mwb.handle_event(info_b.to_event(QEvent.KeyRelease)) + assert not res + assert 0 == len(mwb._partial_match_events) + assert 0 == len(mwb._releaseevents_to_pass) diff --git a/tests/unit/keyinput/test_modeparsers.py b/tests/unit/keyinput/test_modeparsers.py index 88f959b74..8369c7d90 100644 --- a/tests/unit/keyinput/test_modeparsers.py +++ b/tests/unit/keyinput/test_modeparsers.py @@ -194,6 +194,7 @@ class TestHintKeyParser: signals = [command_parser.forward_partial_key] * len(seq) with qtbot.wait_signals(signals) as blocker: handle_text(keyparser, seq[-1]) + assert blocker.signal_triggered assert forward_partial_key.call_args_list == [ ((str(keyutils.KeyInfo(key, Qt.KeyboardModifier.NoModifier)),),) for key in seq ] @@ -245,6 +246,7 @@ class TestHintKeyParser: signals = [command_parser.forward_partial_key] * len(seq) with qtbot.wait_signals(signals) as blocker: handle_text(keyparser, Qt.Key.Key_F) + assert blocker.signal_triggered assert forward_partial_key.call_args_list == [ ((str(keyutils.KeyInfo(key, Qt.KeyboardModifier.NoModifier)),),) for key in seq ] @@ -290,6 +292,7 @@ class TestHintKeyParser: timer = keyparser._partial_timer assert not timer.isActive() + behavior = None for key, behavior in data_sequence: keyinfo = keyutils.KeyInfo(key, Qt.KeyboardModifier.NoModifier) if behavior == 'timer_active': @@ -315,8 +318,7 @@ class TestHintKeyParser: assert (timeout - (timeout//4)) < timer.remainingTime() assert timer.isActive() else: - # Unreachable - assert False + pytest.fail('Unreachable') if behavior in ['timer_active', 'timer_reset']: # Now simulate a timeout and check the keystring has been forwarded. with qtbot.wait_signal(command_parser.keystring_updated) as blocker: