From 193a8d524269f05ddc13f7ebf3f497242a551bb1 Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Sat, 22 Aug 2015 23:26:13 +0200 Subject: [PATCH 1/8] Add unit tests for KeyInput.BaseKeyParser --- qutebrowser/keyinput/basekeyparser.py | 5 ++--- tests/unit/keyinput/test_basekeyparser.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/qutebrowser/keyinput/basekeyparser.py b/qutebrowser/keyinput/basekeyparser.py index 56b9cfaac..2b41dac65 100644 --- a/qutebrowser/keyinput/basekeyparser.py +++ b/qutebrowser/keyinput/basekeyparser.py @@ -186,9 +186,6 @@ class BaseKeyParser(QObject): match, binding = self._match_key(cmd_input) - if not isinstance(match, self.Match): - raise TypeError("Value {} is no Match member!".format(match)) - if match == self.Match.definitive: self._debug_log("Definitive match for '{}'.".format( self._keystring)) @@ -205,6 +202,8 @@ class BaseKeyParser(QObject): self._debug_log("Giving up with '{}', no matches".format( self._keystring)) self._keystring = '' + else: + raise AssertionError("Invalid match value {!r}".format(match)) return match def _match_key(self, cmd_input): diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index d5eadbb5e..9abba0e9c 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -27,9 +27,11 @@ from PyQt5.QtCore import Qt import pytest from qutebrowser.keyinput import basekeyparser +from qutebrowser.utils import utils CONFIG = {'input': {'timeout': 100}} +CONFIG_NO_TIMEOUT = {'input': {'timeout': 0}} @pytest.fixture @@ -131,6 +133,15 @@ class TestSpecialKeys: self.kp.handle(fake_keyevent_factory(Qt.Key_A)) assert not self.kp.execute.called + def test_no_binding(self, monkeypatch, fake_keyevent_factory): + """Test special key with no binding.""" + def none_return(binding): + return None + + monkeypatch.setattr(utils, 'keyevent_to_string', none_return) + self.kp.handle(fake_keyevent_factory(Qt.Key_A, Qt.NoModifier)) + assert not self.kp.execute.called + @pytest.mark.usefixtures('mock_timer') class TestKeyChain: @@ -205,6 +216,17 @@ class TestKeyChain: assert not timer.isActive() assert self.kp._keystring == '' + def test_ambiguous_keychain_no_timeout(self, fake_keyevent_factory, + config_stub, monkeypatch): + """Test ambiguous keychain with timeout equal to 0.""" + config_stub.data = CONFIG_NO_TIMEOUT + monkeypatch.setattr('qutebrowser.keyinput.basekeyparser.config', + config_stub) + self.kp.handle(fake_keyevent_factory(Qt.Key_A, text='a')) + assert self.kp.execute.called + timer = self.kp._ambiguous_timer + assert not timer.isActive() + def test_invalid_keychain(self, fake_keyevent_factory): """Test invalid keychain.""" self.kp.handle(fake_keyevent_factory(Qt.Key_B, text='b')) From af9647221aecec5b195ed8947ccd5c8ebab82e23 Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Mon, 24 Aug 2015 18:12:12 +0200 Subject: [PATCH 2/8] Add tests for BaseKeyParser --- qutebrowser/keyinput/basekeyparser.py | 2 +- tests/unit/keyinput/test_basekeyparser.py | 40 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/qutebrowser/keyinput/basekeyparser.py b/qutebrowser/keyinput/basekeyparser.py index 2b41dac65..a2eef47c7 100644 --- a/qutebrowser/keyinput/basekeyparser.py +++ b/qutebrowser/keyinput/basekeyparser.py @@ -352,7 +352,7 @@ class BaseKeyParser(QObject): def on_keyconfig_changed(self, mode): """Re-read the config if a key binding was changed.""" if self._modename is None: - raise AttributeError("on_keyconfig_changed called but no section " + raise AssertionError("on_keyconfig_changed called but no section " "defined!") if mode == self._modename: self.read_config() diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 9abba0e9c..85fd7953d 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -25,6 +25,7 @@ from unittest import mock from PyQt5.QtCore import Qt import pytest +import pytestqt from qutebrowser.keyinput import basekeyparser from qutebrowser.utils import utils @@ -92,6 +93,29 @@ class TestReadConfig: assert 'foo' in kp.bindings assert 'ctrl+x' in kp.special_bindings + def test_on_keyconfig_changed(self): + """Test the handling of changes in the config.""" + kp = basekeyparser.BaseKeyParser(0, supports_count=False, + supports_chains=False) + kp.read_config = mock.Mock() + kp._modename = None + + # No config set so self._modename is None + with pytest.raises(AssertionError) as excinfo: + kp.on_keyconfig_changed("normal") + assert "on_keyconfig_changed called but no section defined!" in str( + excinfo.value) + assert not kp.read_config.called + + kp._modename = "normal" + kp.on_keyconfig_changed("normal2") + # Modenames are not equal so read_config() should not be called + assert not kp.read_config.called + + kp.on_keyconfig_changed("normal") + # Both modenames equal so read_config() should be called + assert kp.read_config.called + @pytest.mark.usefixtures('mock_timer') class TestSpecialKeys: @@ -233,6 +257,22 @@ class TestKeyChain: self.kp.handle(fake_keyevent_factory(Qt.Key_C, text='c')) assert self.kp._keystring == '' + def test_delayed_exec(self, fake_keyevent_factory, config_stub, + monkeypatch, qtbot): + """Test delayec execute for ambiguous keychain.""" + config_stub.data = CONFIG + monkeypatch.setattr('qutebrowser.keyinput.basekeyparser.config', + config_stub) + timer = self.kp._ambiguous_timer + # 'a' is an ambiguous result. + self.kp.handle(fake_keyevent_factory(Qt.Key_A, text='a')) + assert not self.kp.execute.called + assert timer.isActive() + # We wait for the timeout to occur. + with qtbot.waitSignal(self.kp.keystring_updated, raising=True): + pass + assert self.kp.execute.called + @pytest.mark.usefixtures('mock_timer') class TestCount: From 1a227ae3a7b5ec1e0f02010a3e90bcce23da3434 Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Mon, 24 Aug 2015 19:29:50 +0200 Subject: [PATCH 3/8] pytestqt is not required --- tests/unit/keyinput/test_basekeyparser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 85fd7953d..a8400f6e0 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -25,7 +25,6 @@ from unittest import mock from PyQt5.QtCore import Qt import pytest -import pytestqt from qutebrowser.keyinput import basekeyparser from qutebrowser.utils import utils From 05eb9bd08c9d3b8c6dc420eb9efe4eafda7cf7a2 Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Tue, 25 Aug 2015 10:28:46 +0200 Subject: [PATCH 4/8] Remove unnecessary lines --- tests/unit/keyinput/test_basekeyparser.py | 52 +++++++++++++---------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index a8400f6e0..50be9aefd 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -92,26 +92,30 @@ class TestReadConfig: assert 'foo' in kp.bindings assert 'ctrl+x' in kp.special_bindings - def test_on_keyconfig_changed(self): - """Test the handling of changes in the config.""" + def test_on_keyconfig_changed_mode_none(self): + """Test the changes in config with _modename = None.""" kp = basekeyparser.BaseKeyParser(0, supports_count=False, supports_chains=False) - kp.read_config = mock.Mock() - kp._modename = None + assert kp._modename is None # No config set so self._modename is None with pytest.raises(AssertionError) as excinfo: - kp.on_keyconfig_changed("normal") - assert "on_keyconfig_changed called but no section defined!" in str( - excinfo.value) - assert not kp.read_config.called + kp.on_keyconfig_changed('normal') + expected_text = "on_keyconfig_changed called but no section defined!" + assert str(excinfo.value) == expected_text - kp._modename = "normal" - kp.on_keyconfig_changed("normal2") + def test_on_keyconfig_changed_mode_normal(self): + """Test the changes in config with _modename set.""" + kp = basekeyparser.BaseKeyParser(0, supports_count=False, + supports_chains=False) + kp.read_config = mock.Mock() + + kp._modename = 'normal' + kp.on_keyconfig_changed('normal2') # Modenames are not equal so read_config() should not be called assert not kp.read_config.called - kp.on_keyconfig_changed("normal") + kp.on_keyconfig_changed('normal') # Both modenames equal so read_config() should be called assert kp.read_config.called @@ -158,10 +162,7 @@ class TestSpecialKeys: def test_no_binding(self, monkeypatch, fake_keyevent_factory): """Test special key with no binding.""" - def none_return(binding): - return None - - monkeypatch.setattr(utils, 'keyevent_to_string', none_return) + monkeypatch.setattr(utils, 'keyevent_to_string', lambda binding: None) self.kp.handle(fake_keyevent_factory(Qt.Key_A, Qt.NoModifier)) assert not self.kp.execute.called @@ -243,12 +244,9 @@ class TestKeyChain: config_stub, monkeypatch): """Test ambiguous keychain with timeout equal to 0.""" config_stub.data = CONFIG_NO_TIMEOUT - monkeypatch.setattr('qutebrowser.keyinput.basekeyparser.config', - config_stub) self.kp.handle(fake_keyevent_factory(Qt.Key_A, text='a')) assert self.kp.execute.called - timer = self.kp._ambiguous_timer - assert not timer.isActive() + assert not self.kp._ambiguous_timer.isActive() def test_invalid_keychain(self, fake_keyevent_factory): """Test invalid keychain.""" @@ -260,13 +258,11 @@ class TestKeyChain: monkeypatch, qtbot): """Test delayec execute for ambiguous keychain.""" config_stub.data = CONFIG - monkeypatch.setattr('qutebrowser.keyinput.basekeyparser.config', - config_stub) - timer = self.kp._ambiguous_timer + # 'a' is an ambiguous result. self.kp.handle(fake_keyevent_factory(Qt.Key_A, text='a')) assert not self.kp.execute.called - assert timer.isActive() + assert self.kp._ambiguous_timer.isActive() # We wait for the timeout to occur. with qtbot.waitSignal(self.kp.keystring_updated, raising=True): pass @@ -329,3 +325,13 @@ class TestCount: self.kp.handle(fake_keyevent_factory(Qt.Key_A, text='c')) self.kp.execute.assert_called_once_with('ccc', self.kp.Type.chain, 23) assert self.kp._keystring == '' + + +def test_clear_keystring(qtbot): + """Test that the keystring is cleared and the signal is emitted""" + kp = basekeyparser.BaseKeyParser(0) + kp._keystring = 'test' + kp.clear_keystring() + with qtbot.waitSignal(kp.keystring_updated): + pass + assert kp._keystring is '' From 09161faca599dd309fb92577a63d5151772dd285 Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Tue, 25 Aug 2015 12:04:22 +0200 Subject: [PATCH 5/8] Refactor read_config for easier testing --- qutebrowser/keyinput/basekeyparser.py | 26 +++++++++++++---------- tests/unit/keyinput/test_basekeyparser.py | 7 ++++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/qutebrowser/keyinput/basekeyparser.py b/qutebrowser/keyinput/basekeyparser.py index a2eef47c7..754f6672f 100644 --- a/qutebrowser/keyinput/basekeyparser.py +++ b/qutebrowser/keyinput/basekeyparser.py @@ -326,17 +326,21 @@ class BaseKeyParser(QObject): self.special_bindings = {} keyconfparser = objreg.get('key-config') for (key, cmd) in keyconfparser.get_bindings_for(modename).items(): - if not cmd: - continue - elif key.startswith('<') and key.endswith('>'): - keystr = utils.normalize_keystr(key[1:-1]) - self.special_bindings[keystr] = cmd - elif self._supports_chains: - self.bindings[key] = cmd - elif self._warn_on_keychains: - log.keyboard.warning( - "Ignoring keychain '{}' in mode '{}' because " - "keychains are not supported there.".format(key, modename)) + if cmd: + self._parse_key_command(modename, key, cmd) + + def _parse_key_command(self, modename, key, cmd): + """Parse the keys and their command and store them in the object.""" + if key.startswith('<') and key.endswith('>'): + keystr = utils.normalize_keystr(key[1:-1]) + self.special_bindings[keystr] = cmd + elif self._supports_chains: + self.bindings[key] = cmd + elif self._warn_on_keychains: + log.keyboard.warning( + "Ignoring keychain '{}' in mode '{}' because " + "keychains are not supported there." + .format(key, modename)) def execute(self, cmdstr, keytype, count=None): """Handle a completed keychain. diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 50be9aefd..0ebab9f9b 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -79,6 +79,13 @@ class TestReadConfig: with pytest.raises(ValueError): kp.read_config() + def test_read_config_no_modename(self): + """Test reading config with _modename set.""" + kp = basekeyparser.BaseKeyParser(0, supports_chains=True) + kp._modename = "normal" + kp.read_config(modename=None) + assert 'a' in kp.bindings + def test_read_config_valid(self): """Test reading config.""" kp = basekeyparser.BaseKeyParser(0, supports_count=True, From f54295f95ca7e516e79b273d45e8b1117e621bdb Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Tue, 25 Aug 2015 16:28:02 +0200 Subject: [PATCH 6/8] Test _warn_on_keychains. --- tests/unit/keyinput/test_basekeyparser.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 0ebab9f9b..acbacce91 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -126,6 +126,22 @@ class TestReadConfig: # Both modenames equal so read_config() should be called assert kp.read_config.called + def test_warn_on_keychains(self, fake_keyevent_factory, monkeypatch): + """Test _warn_on_keychains.""" + kp = basekeyparser.BaseKeyParser(0, supports_count=False, + supports_chains=False) + + log_mock = mock.Mock() + monkeypatch.setattr('qutebrowser.utils.log.keyboard', log_mock) + + kp._warn_on_keychains = False + kp.read_config('normal') + assert not log_mock.warning.called + + kp._warn_on_keychains = True + kp.read_config('normal') + assert log_mock.warning.called + @pytest.mark.usefixtures('mock_timer') class TestSpecialKeys: From b4f4c97cf90ac5bace07fae405c3fb46f13757df Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Wed, 26 Aug 2015 01:22:09 +0200 Subject: [PATCH 7/8] Add basekeyparser.py to PERFECT_FILES --- scripts/dev/check_coverage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index f5fefaf76..19a719fa3 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -46,6 +46,8 @@ PERFECT_FILES = [ 'qutebrowser/browser/network/networkreply.py', 'qutebrowser/browser/signalfilter.py', + 'qutebrowser/keyinput/basekeyparser.py', + 'qutebrowser/misc/readline.py', 'qutebrowser/misc/split.py', 'qutebrowser/misc/msgbox.py', From cd34fc4b57154983c6564d8b5542eefea655c8cc Mon Sep 17 00:00:00 2001 From: Alexander Cogneau Date: Wed, 26 Aug 2015 12:13:47 +0200 Subject: [PATCH 8/8] Small changes to basekeyparser tests --- tests/unit/keyinput/test_basekeyparser.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index acbacce91..36cf0025b 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -354,7 +354,6 @@ def test_clear_keystring(qtbot): """Test that the keystring is cleared and the signal is emitted""" kp = basekeyparser.BaseKeyParser(0) kp._keystring = 'test' - kp.clear_keystring() with qtbot.waitSignal(kp.keystring_updated): - pass - assert kp._keystring is '' + kp.clear_keystring() + assert kp._keystring == ''