Merge remote-tracking branch 'origin/pr/3702'

qutebrowser/browser/webengine/spell.py

@@ -24,16 +24,18 @@ import os
 import re
 
 from PyQt5.QtCore import QLibraryInfo
-from qutebrowser.utils import log
+from qutebrowser.utils import log, message
+
+dict_version_re = re.compile(r".+-(?P<version>[0-9]+-[0-9]+?)\.bdic")
 
 
 def version(filename):
     """Extract the version number from the dictionary file name."""
-    version_re = re.compile(r".+-(?P<version>[0-9]+-[0-9]+?)\.bdic")
-    match = version_re.fullmatch(filename)
+    match = dict_version_re.match(filename)
     if match is None:
-        raise ValueError('the given dictionary file name is malformed: {}'
-                         .format(filename))
+        message.warning(
+            "Found a dictionary with a malformed name: {}".format(filename))
+        return None
     return tuple(int(n) for n in match.group('version').split('-'))
 
 
@@ -44,15 +46,23 @@ def dictionary_dir():
 
 
 def local_files(code):
-    """Return all installed dictionaries for the given code."""
+    """Return all installed dictionaries for the given code.
+
+    The returned dictionaries are sorted by version, therefore the latest will
+    be the first element. The list will be empty if no dictionaries are found.
+    """
     pathname = os.path.join(dictionary_dir(), '{}*.bdic'.format(code))
     matching_dicts = glob.glob(pathname)
-    files = []
-    for matching_dict in sorted(matching_dicts, key=version, reverse=True):
-        filename = os.path.basename(matching_dict)
-        log.config.debug('Found file for dict {}: {}'.format(code, filename))
-        files.append(filename)
-    return files
+    versioned_dicts = []
+    for matching_dict in matching_dicts:
+        parsed_version = version(matching_dict)
+        if parsed_version is not None:
+            filename = os.path.basename(matching_dict)
+            log.config.debug('Found file for dict {}: {}'
+                             .format(code, filename))
+            versioned_dicts.append((parsed_version, filename))
+    return [filename for version, filename
+            in sorted(versioned_dicts, reverse=True)]
 
 
 def local_filename(code):

tests/unit/browser/webengine/test_spell.py

@@ -17,31 +17,69 @@
 # You should have received a copy of the GNU General Public License
 # along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
 
-import pytest
+"""Tests for qutebrowser.browser.webengine.spell module."""
+
+import logging
+import os
+
+from PyQt5.QtCore import QLibraryInfo
 from qutebrowser.browser.webengine import spell
+from qutebrowser.utils import usertypes
 
 
-def test_version():
+def test_version(message_mock, caplog):
+    """Tests parsing dictionary version from its file name."""
     assert spell.version('en-US-8-0.bdic') == (8, 0)
     assert spell.version('pl-PL-3-0.bdic') == (3, 0)
-    with pytest.raises(ValueError):
-        spell.version('malformed_filename')
+    with caplog.at_level(logging.WARNING):
+        assert spell.version('malformed_filename') is None
+    msg = message_mock.getmsg(usertypes.MessageLevel.warning)
+    expected = ("Found a dictionary with a malformed name: malformed_filename")
+    assert msg.text == expected
 
 
-def test_local_filename_dictionary_does_not_exist(tmpdir, monkeypatch):
+def test_dictionary_dir(monkeypatch):
+    monkeypatch.setattr(QLibraryInfo, 'location', lambda _: 'datapath')
+    assert spell.dictionary_dir() == os.path.join('datapath',
+                                                  'qtwebengine_dictionaries')
+
+
+def test_local_filename_dictionary_does_not_exist(monkeypatch):
+    """Tests retrieving local filename when the dir doesn't exits."""
     monkeypatch.setattr(
         spell, 'dictionary_dir', lambda: '/some-non-existing-dir')
     assert not spell.local_filename('en-US')
 
 
 def test_local_filename_dictionary_not_installed(tmpdir, monkeypatch):
+    """Tests retrieving local filename when the dict not installed."""
     monkeypatch.setattr(spell, 'dictionary_dir', lambda: str(tmpdir))
     assert not spell.local_filename('en-US')
 
 
+def test_local_filename_not_installed_malformed(tmpdir, monkeypatch, caplog):
+    """Tests retrieving local filename when the only file is malformed."""
+    monkeypatch.setattr(spell, 'dictionary_dir', lambda: str(tmpdir))
+    (tmpdir / 'en-US.bdic').ensure()
+    with caplog.at_level(logging.WARNING):
+        assert not spell.local_filename('en-US')
+
+
 def test_local_filename_dictionary_installed(tmpdir, monkeypatch):
+    """Tests retrieving local filename when the dict installed."""
     monkeypatch.setattr(spell, 'dictionary_dir', lambda: str(tmpdir))
     for lang_file in ['en-US-11-0.bdic', 'en-US-7-1.bdic', 'pl-PL-3-0.bdic']:
         (tmpdir / lang_file).ensure()
     assert spell.local_filename('en-US') == 'en-US-11-0'
     assert spell.local_filename('pl-PL') == 'pl-PL-3-0'
+
+
+def test_local_filename_installed_malformed(tmpdir, monkeypatch, caplog):
+    """Tests retrieving local filename when the dict installed.
+
+    In this usecase, another existing file is malformed."""
+    monkeypatch.setattr(spell, 'dictionary_dir', lambda: str(tmpdir))
+    for lang_file in ['en-US-11-0.bdic', 'en-US-7-1.bdic', 'en-US.bdic']:
+        (tmpdir / lang_file).ensure()
+    with caplog.at_level(logging.WARNING):
+        assert spell.local_filename('en-US') == 'en-US-11-0'