Make various SQL code review changes.

- Fix outdated comments
- Use mock specs when possible
- More precise error message check in test_import_txt_invalid.
- Fix copyright message
- Tweak missing pyqt error message
- Dead code: remove group_by and where from sqlcategory.
  With the new separate completion table, these are no longer used.
- Move test_history out of webkit/. History is no longer purely webkit
  related, it could be webengine.

qutebrowser/browser/qutescheme.py

@@ -194,7 +194,6 @@ def history_data(start_time, offset=None):
     """
     hist = objreg.get('web-history')
     if offset is not None:
-        # end is 24hrs earlier than start
         entries = hist.entries_before(start_time, limit=1000, offset=offset)
     else:
         # end is 24hrs earlier than start

qutebrowser/completion/models/listcategory.py

@@ -36,7 +36,7 @@ class ListCategory(QSortFilterProxyModel):
         super().__init__(parent)
         self.name = name
         self.srcmodel = QStandardItemModel(parent=self)
-        self.pattern = ''
+        self._pattern = ''
         # ListCategory filters all columns
         self.columns_to_filter = [0, 1, 2]
         self.setFilterKeyColumn(-1)
@@ -51,7 +51,7 @@ class ListCategory(QSortFilterProxyModel):
         Args:
             val: The value to set.
         """
-        self.pattern = val
+        self._pattern = val
         val = re.sub(r' +', r' ', val)  # See #1919
         val = re.escape(val)
         val = val.replace(r'\ ', '.*')
@@ -64,7 +64,7 @@ class ListCategory(QSortFilterProxyModel):
     def lessThan(self, lindex, rindex):
         """Custom sorting implementation.
 
-        Prefers all items which start with self.pattern. Other than that, uses
+        Prefers all items which start with self._pattern. Other than that, uses
         normal Python string sorting.
 
         Args:
@@ -80,8 +80,8 @@ class ListCategory(QSortFilterProxyModel):
         left = self.srcmodel.data(lindex)
         right = self.srcmodel.data(rindex)
 
-        leftstart = left.startswith(self.pattern)
+        leftstart = left.startswith(self._pattern)
-        rightstart = right.startswith(self.pattern)
+        rightstart = right.startswith(self._pattern)
 
         if leftstart and rightstart:
             return left < right

qutebrowser/completion/models/sqlcategory.py

@@ -33,7 +33,7 @@ class SqlCategory(QSqlQueryModel):
     """Wraps a SqlQuery for use as a completion category."""
 
     def __init__(self, name, *, title=None, filter_fields, sort_by=None,
-                 sort_order=None, select='*', where=None, group_by=None,
+                 sort_order=None, select='*',
                  delete_func=None, parent=None):
         """Create a new completion category backed by a sql table.
 
@@ -42,7 +42,6 @@ class SqlCategory(QSqlQueryModel):
             title: Title of category, defaults to table name.
             filter_fields: Names of fields to apply filter pattern to.
             select: A custom result column expression for the select statement.
-            where: An optional clause to filter out some rows.
             sort_by: The name of the field to sort by, or None for no sorting.
             sort_order: Either 'asc' or 'desc', if sort_by is non-None
             delete_func: Callback to delete a selected item.
@@ -57,10 +56,6 @@ class SqlCategory(QSqlQueryModel):
                                 for f in filter_fields)
         querystr += ')'
 
-        if where:
-            querystr += ' and ' + where
-        if group_by:
-            querystr += ' group by {}'.format(group_by)
         if sort_by:
             assert sort_order in ['asc', 'desc'], sort_order
             querystr += ' order by {} {}'.format(sort_by, sort_order)

qutebrowser/misc/earlyinit.py

@@ -337,12 +337,7 @@ def check_libraries(backend):
                                  "or Install via pip.",
                          pip="PyYAML"),
         'PyQt5.QtSql':
-            _missing_str("PyQt5.QtSql",
+            _missing_str("PyQt5.QtSql")
-                         windows="Use the installer by Riverbank computing "
-                                 "or the standalone qutebrowser exe. "
-                                 "http://www.riverbankcomputing.co.uk/"
-                                 "software/pyqt/download5",
-                         pip="PyQt5"),
     }
     if backend == 'webengine':
         modules['PyQt5.QtWebEngineWidgets'] = _missing_str("QtWebEngine",

qutebrowser/misc/sql.py

@@ -209,8 +209,8 @@ class SqlTable(QObject):
         """Performantly append multiple rows to the table.
 
         Args:
-            rows: A list of lists, where each sub-list is a row.
             values: A dict with a list of values to insert for each field name.
+            replace: If true, overwrite rows with a primary key match.
         """
         q = self._insert_query(values, replace)
         for key, val in values.items():

tests/unit/browser/test_history.py

@@ -113,7 +113,8 @@ def test_clear(qtbot, tmpdir, hist, mocker):
     hist.add_url(QUrl('http://example.com/'))
     hist.add_url(QUrl('http://www.qutebrowser.org/'))
 
-    m = mocker.patch('qutebrowser.browser.history.message.confirm_async')
+    m = mocker.patch('qutebrowser.browser.history.message.confirm_async',
+                     spec=[])
     hist.clear()
     assert m.called
 
@@ -288,15 +289,19 @@ def test_import_txt_skip(hist, data_tmpdir, line, monkeypatch, stubs):
     '68891-x http://example.com/bad-flag',
     '68891 http://.com',
 ])
-def test_import_txt_invalid(hist, data_tmpdir, line, monkeypatch, stubs):
+def test_import_txt_invalid(hist, data_tmpdir, line, monkeypatch, stubs,
+                            caplog):
     """import_txt should fail on certain lines."""
     monkeypatch.setattr(history, 'QTimer', stubs.InstaTimer)
     histfile = data_tmpdir / 'history'
     histfile.write(line)
 
-    with pytest.raises(Exception):
+    with caplog.at_level(logging.ERROR):
         hist.import_txt()
 
+    assert any(rec.msg.startswith("Failed to import history:")
+               for rec in caplog.records)
+
     assert histfile.exists()
 
 

tests/unit/completion/test_completionmodel.py

@@ -36,7 +36,7 @@ def test_first_last_item(counts):
     """Test that first() and last() index to the first and last items."""
     model = completionmodel.CompletionModel()
     for c in counts:
-        cat = mock.Mock()
+        cat = mock.Mock(spec=['layoutChanged'])
         cat.rowCount = mock.Mock(return_value=c)
         model.add_category(cat)
     nonempty = [i for i, rowCount in enumerate(counts) if rowCount > 0]
@@ -70,16 +70,17 @@ def test_count(counts):
 def test_set_pattern(pat):
     """Validate the filtering and sorting results of set_pattern."""
     model = completionmodel.CompletionModel()
-    cats = [mock.Mock(spec=['set_pattern', 'layoutChanged'])] * 3
+    cats = [mock.Mock(spec=['set_pattern', 'layoutChanged']) for _ in range(3)]
     for c in cats:
-        c.set_pattern = mock.Mock()
+        c.set_pattern = mock.Mock(spec=[])
         model.add_category(c)
     model.set_pattern(pat)
-    assert all(c.set_pattern.called_with([pat]) for c in cats)
+    for c in cats:
+        c.set_pattern.assert_called_with(pat)
 
 
 def test_delete_cur_item():
-    func = mock.Mock()
+    func = mock.Mock(spec=[])
     model = completionmodel.CompletionModel()
     cat = listcategory.ListCategory('', [('foo', 'bar')], delete_func=func)
     model.add_category(cat)

tests/unit/completion/test_completionwidget.py

@@ -83,7 +83,7 @@ def test_set_model(completionview):
 
 def test_set_pattern(completionview):
     model = completionmodel.CompletionModel()
-    model.set_pattern = mock.Mock()
+    model.set_pattern = mock.Mock(spec=[])
     completionview.set_model(model)
     completionview.set_pattern('foo')
     model.set_pattern.assert_called_with('foo')
@@ -214,7 +214,7 @@ def test_completion_show(show, rows, quick_complete, completionview,
 
 def test_completion_item_del(completionview):
     """Test that completion_item_del invokes delete_cur_item in the model."""
-    func = mock.Mock()
+    func = mock.Mock(spec=[])
     model = completionmodel.CompletionModel()
     cat = listcategory.ListCategory('', [('foo', 'bar')], delete_func=func)
     model.add_category(cat)
@@ -226,7 +226,7 @@ def test_completion_item_del(completionview):
 
 def test_completion_item_del_no_selection(completionview):
     """Test that completion_item_del with an invalid index."""
-    func = mock.Mock()
+    func = mock.Mock(spec=[])
     model = completionmodel.CompletionModel()
     cat = listcategory.ListCategory('', [('foo',)], delete_func=func)
     model.add_category(cat)

tests/unit/completion/test_listcategory.py

@@ -1,6 +1,6 @@
 # vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
 
-# Copyright 2015-2016 Florian Bruhin (The Compiler) <mail@qutebrowser.org>
+# Copyright 2017 Ryan Roden-Corrent (rcorre) <ryan@rcorre.net>
 #
 # This file is part of qutebrowser.
 #

tests/unit/completion/test_sqlcategory.py

@@ -135,32 +135,11 @@ def test_select():
     utils.validate_model(cat, [('bar', 'baz', 'foo')])
 
 
-def test_where():
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
-    table.insert({'a': 'foo', 'b': 'bar', 'c': False})
-    table.insert({'a': 'baz', 'b': 'biz', 'c': True})
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], where='not c')
-    cat.set_pattern('')
-    utils.validate_model(cat, [('foo', 'bar', False)])
-
-
-def test_group():
-    table = sql.SqlTable('Foo', ['a', 'b'])
-    table.insert({'a': 'foo', 'b': 1})
-    table.insert({'a': 'bar', 'b': 3})
-    table.insert({'a': 'foo', 'b': 2})
-    table.insert({'a': 'bar', 'b': 0})
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'],
-                                  select='a, max(b)', group_by='a')
-    cat.set_pattern('')
-    utils.validate_model(cat, [('bar', 3), ('foo', 2)])
-
-
 def test_delete_cur_item():
     table = sql.SqlTable('Foo', ['a', 'b'])
     table.insert({'a': 'foo', 'b': 1})
     table.insert({'a': 'bar', 'b': 2})
-    func = unittest.mock.MagicMock()
+    func = unittest.mock.Mock(spec=[])
     cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], delete_func=func)
     cat.set_pattern('')
     cat.delete_cur_item(cat.index(0, 0))

tests/unit/misc/test_lineparser.py

@@ -58,7 +58,8 @@ class TestBaseLineParser:
         mocker.patch('builtins.open', mock.mock_open())
 
         with lineparser._open('r'):
-            with pytest.raises(IOError):
+            with pytest.raises(IOError,
+                               match="Refusing to double-open LineParser."):
                 with lineparser._open('r'):
                     pass