Refactor qtutils.version_check API

Fixes #2423

7 files changed, 54 insertions, 43 deletions

diff --git a/.pylintrc b/.pylintrc
index 621e69d00..3f06facaf 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -38,8 +38,7 @@ disable=no-self-use,
         suppressed-message,
         too-many-return-statements,
         duplicate-code,
-        wrong-import-position,
-        too-many-boolean-expressions
+        wrong-import-position
 
 [BASIC]
 function-rgx=[a-z_][a-z0-9_]{2,50}$
diff --git a/qutebrowser/misc/earlyinit.py b/qutebrowser/misc/earlyinit.py
index 4f4f41c32..3622dc36a 100644
--- a/qutebrowser/misc/earlyinit.py
+++ b/qutebrowser/misc/earlyinit.py
@@ -34,7 +34,6 @@ import sys
 import faulthandler
 import traceback
 import signal
-import operator
 import importlib
 import pkg_resources
 import datetime
@@ -264,14 +263,14 @@ def check_qt_version(backend):
     """Check if the Qt version is recent enough."""
     from PyQt5.QtCore import PYQT_VERSION, PYQT_VERSION_STR
     from qutebrowser.utils import qtutils, version
-    if (qtutils.version_check('5.2.0', operator.lt, strict=True) or
+    if (not qtutils.version_check('5.2.0', strict=True) or
             PYQT_VERSION < 0x050200):
         text = ("Fatal error: Qt and PyQt >= 5.2.0 are required, but Qt {} / "
                 "PyQt {} is installed.".format(version.qt_version(),
                                                PYQT_VERSION_STR))
         _die(text)
     elif (backend == 'webengine' and (
-            qtutils.version_check('5.7.1', operator.lt, strict=True) or
+            not qtutils.version_check('5.7.1', strict=True) or
             PYQT_VERSION < 0x050700)):
         text = ("Fatal error: Qt >= 5.7.1 and PyQt >= 5.7 are required for "
                 "QtWebEngine support, but Qt {} / PyQt {} is installed."
diff --git a/qutebrowser/misc/miscwidgets.py b/qutebrowser/misc/miscwidgets.py
index bbfbbc585..9983935f4 100644
--- a/qutebrowser/misc/miscwidgets.py
+++ b/qutebrowser/misc/miscwidgets.py
@@ -262,7 +262,7 @@ class WrapperLayout(QLayout):
         self._widget = widget
         container.setFocusProxy(widget)
         widget.setParent(container)
-        if (qtutils.version_check('5.8.0', op=operator.eq) and
+        if (qtutils.version_check('5.8.0', exact=True) and
                 objects.backend == usertypes.Backend.QtWebEngine and
                 container.window() and
                 container.window().windowHandle() and
diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py
index 6fc5956a6..e36fd0ffb 100644
--- a/qutebrowser/utils/qtutils.py
+++ b/qutebrowser/utils/qtutils.py
@@ -80,20 +80,22 @@ class QtOSError(OSError):
             self.qt_errno = None
 
 
-def version_check(version, op=operator.ge, strict=False):
+def version_check(version, exact=False, strict=False):
     """Check if the Qt runtime version is the version supplied or newer.
 
     Args:
         version: The version to check against.
-        op: The operator to use for the check.
+        exact: if given, check with == instead of >=
         strict: If given, also check the compiled Qt version.
     """
-    if strict:
-        assert op in [operator.ge, operator.lt], op
+    # Catch code using the old API for this
+    assert exact not in [operator.gt, operator.lt, operator.ge, operator.le,
+                         operator.eq], exact
     parsed = pkg_resources.parse_version(version)
+    op = operator.eq if exact else operator.ge
     result = op(pkg_resources.parse_version(qVersion()), parsed)
-    if ((strict and op == operator.ge and result) or
-            (strict and op == operator.lt and not result)):
+    if strict and result:
+        # v1 ==/>= parsed, now check if v2 ==/>= parsed too.
         result = op(pkg_resources.parse_version(QT_VERSION_STR), parsed)
     return result
 
@@ -160,7 +162,10 @@ def get_args(namespace):
 def check_print_compat():
     """Check if printing should work in the given Qt version."""
     # WORKAROUND (remove this when we bump the requirements to 5.3.0)
-    return not (os.name == 'nt' and version_check('5.3.0', operator.lt))
+    if os.name == 'nt':
+        return version_check('5.3')
+    else:
+        return True
 
 
 def ensure_valid(obj):
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index d0538f89a..e658cbf8f 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -169,7 +169,7 @@ def actute_warning():
         return
     # Qt >= 5.3 doesn't seem to be affected
     try:
-        if qtutils.version_check('5.3.0'):
+        if qtutils.version_check('5.3'):
             return
     except ValueError:  # pragma: no cover
         pass
diff --git a/tests/end2end/conftest.py b/tests/end2end/conftest.py
index 939a377f7..42ce99429 100644
--- a/tests/end2end/conftest.py
+++ b/tests/end2end/conftest.py
@@ -68,7 +68,7 @@ def _get_version_tag(tag):
     """
     version_re = re.compile(r"""
         (?P<package>qt|pyqt)
-        (?P<operator>==|>|>=|<|<=|!=)
+        (?P<operator>==|>=|!=)
         (?P<version>\d+\.\d+(\.\d+)?)
     """, re.VERBOSE)
 
@@ -76,23 +76,24 @@ def _get_version_tag(tag):
     if not match:
         return None
 
-    operators = {
-        '==': operator.eq,
-        '>': operator.gt,
-        '<': operator.lt,
-        '>=': operator.ge,
-        '<=': operator.le,
-        '!=': operator.ne,
-    }
-
     package = match.group('package')
-    op = operators[match.group('operator')]
     version = match.group('version')
 
     if package == 'qt':
-        return pytest.mark.skipif(not qtutils.version_check(version, op),
-                                  reason='Needs ' + tag)
+        op = match.group('operator')
+        do_skip = {
+            '==': qtutils.version_check(version, exact=True),
+            '>=': qtutils.version_check(version),
+            '!=': not qtutils.version_check(version, exact=True),
+        }
+        return pytest.mark.skipif(do_skip[op], reason='Needs ' + tag)
     elif package == 'pyqt':
+        operators = {
+            '==': operator.eq,
+            '>=': operator.ge,
+            '!=': operator.ne,
+        }
+        op = operators[match.group('operator')]
         major, minor, patch = [int(e) for e in version.split('.')]
         hex_version = (major << 16) | (minor << 8) | patch
         return pytest.mark.skipif(not op(PYQT_VERSION, hex_version),
diff --git a/tests/unit/utils/test_qtutils.py b/tests/unit/utils/test_qtutils.py
index 7cc0742f5..e49712a23 100644
--- a/tests/unit/utils/test_qtutils.py
+++ b/tests/unit/utils/test_qtutils.py
@@ -42,21 +42,28 @@ from qutebrowser.utils import qtutils
 import overflow_test_cases
 
 
-@pytest.mark.parametrize('qversion, compiled, version, op, expected', [
-    ('5.4.0', None, '5.4.0', operator.ge, True),
-    ('5.4.0', None, '5.4.0', operator.eq, True),
-    ('5.4.0', None, '5.4', operator.eq, True),
-    ('5.4.1', None, '5.4', operator.ge, True),
-    ('5.3.2', None, '5.4', operator.ge, False),
-    ('5.3.0', None, '5.3.2', operator.ge, False),
-    # strict=True
-    ('5.4.0', '5.3.0', '5.4.0', operator.ge, False),
-    ('5.4.0', '5.4.0', '5.4.0', operator.ge, True),
-
-    ('5.4.0', '5.3.0', '5.4.0', operator.lt, True),
-    ('5.4.0', '5.4.0', '5.4.0', operator.lt, False),
+@pytest.mark.parametrize('qversion, compiled, version, exact, expected', [
+    # equal versions
+    ('5.4.0', None, '5.4.0', False, True),
+    ('5.4.0', None, '5.4.0', True, True),  # exact=True
+    ('5.4.0', None, '5.4', True, True),  # without trailing 0
+    # newer version installed
+    ('5.4.1', None, '5.4', False, True),
+    ('5.4.1', None, '5.4', True, False),  # exact=True
+    # older version installed
+    ('5.3.2', None, '5.4', False, False),
+    ('5.3.0', None, '5.3.2', False, False),
+    ('5.3.0', None, '5.3.2', True, False),  # exact=True
+    # strict
+    ('5.4.0', '5.3.0', '5.4.0', False, False),
+    ('5.4.0', '5.4.0', '5.4.0', False, True),
+    # strict and exact=True
+    ('5.4.0', '5.5.0', '5.4.0', True, False),
+    ('5.5.0', '5.4.0', '5.4.0', True, False),
+    ('5.4.0', '5.4.0', '5.4.0', True, True),
 ])
-def test_version_check(monkeypatch, qversion, compiled, version, op, expected):
+def test_version_check(monkeypatch, qversion, compiled, version, exact,
+                       expected):
     """Test for version_check().
 
     Args:
@@ -64,7 +71,7 @@ def test_version_check(monkeypatch, qversion, compiled, version, op, expected):
         qversion: The version to set as fake qVersion().
         compiled: The value for QT_VERSION_STR (set strict=True)
         version: The version to compare with.
-        op: The operator to use when comparing.
+        exact: Use exact comparing (==)
         expected: The expected result.
     """
     monkeypatch.setattr(qtutils, 'qVersion', lambda: qversion)
@@ -74,7 +81,7 @@ def test_version_check(monkeypatch, qversion, compiled, version, op, expected):
     else:
         strict = False
 
-    assert qtutils.version_check(version, op, strict=strict) == expected
+    assert qtutils.version_check(version, exact, strict=strict) == expected
 
 
 @pytest.mark.parametrize('version, ng', [