diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 46c7578bd..da66efe24 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -83,8 +83,6 @@ def run(args): standarddir.init(args) if args.version: - # we need to init sql to print the sql version - # we can use an in-memory database as we just want to query the version print(version.version()) sys.exit(usertypes.Exit.ok) @@ -431,8 +429,8 @@ def _init_modules(args, crash_handler): try: sql.init(os.path.join(standarddir.data(), 'history.sqlite')) except sql.SqlException as e: - error.handle_fatal_exc(e, args, 'Is sqlite installed?', - pre_text='Failed to initialize SQL') + error.handle_fatal_exc(e, args, 'Error initializing SQL', + pre_text='Error initializing SQL') sys.exit(usertypes.Exit.err_init) log.init.debug("Initializing web history...") diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index e583fba40..b5b8af149 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -171,6 +171,8 @@ class WebHistory(sql.SqlTable): def _parse_entry(self, line): """Parse a history line like '12345 http://example.com title'.""" + if not line or line.startswith('#'): + return None data = line.split(maxsplit=2) if len(data) == 2: atime, url = data @@ -240,11 +242,8 @@ class WebHistory(sql.SqlTable): data = {'url': [], 'title': [], 'atime': [], 'redirect': []} completion_data = {'url': [], 'title': [], 'last_atime': []} for (i, line) in enumerate(f): - line = line.strip() - if not line or line.startswith('#'): - continue try: - parsed = self._parse_entry(line) + parsed = self._parse_entry(line.strip()) if parsed is None: continue url, title, atime, redirect = parsed @@ -271,10 +270,6 @@ class WebHistory(sql.SqlTable): """ dest = os.path.expanduser(dest) - dirname = os.path.dirname(dest) - if not os.path.exists(dirname): - raise cmdexc.CommandError('Path does not exist', dirname) - lines = ('{}{} {} {}' .format(int(x.atime), '-r' * x.redirect, x.url, x.title) for x in self.select(sort_by='atime', sort_order='asc')) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index c01ae64af..950f21ff1 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -51,12 +51,10 @@ PERFECT_FILES = [ 'browser/webkit/cache.py'), ('tests/unit/browser/webkit/test_cookies.py', 'browser/webkit/cookies.py'), - ('tests/unit/browser/webkit/test_history.py', + ('tests/unit/browser/test_history.py', 'browser/history.py'), - ('tests/unit/browser/webkit/test_history.py', + ('tests/unit/browser/test_history.py', 'browser/webkit/webkithistory.py'), - ('tests/unit/browser/webkit/test_history.py', - 'browser/history.py'), ('tests/unit/browser/webkit/http/test_http.py', 'browser/webkit/http.py'), ('tests/unit/browser/webkit/http/test_content_disposition.py', diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py index 4fd6a0731..dd689a531 100644 --- a/tests/helpers/fixtures.py +++ b/tests/helpers/fixtures.py @@ -257,15 +257,6 @@ def bookmark_manager_stub(stubs): objreg.delete('bookmark-manager') -@pytest.fixture -def web_history_stub(init_sql, stubs): - """Fixture which provides a fake web-history object.""" - stub = stubs.WebHistoryStub() - objreg.register('web-history', stub) - yield stub - objreg.delete('web-history') - - @pytest.fixture def session_manager_stub(stubs): """Fixture which provides a fake session-manager object.""" @@ -491,3 +482,22 @@ def init_sql(data_tmpdir): sql.init(path) yield sql.close() + + +@pytest.fixture +def validate_model(qtmodeltester): + """Provides a function to validate a completion category.""" + def validate(cat, expected): + """Check that a category contains the items in the given order. + + Args: + cat: The category to inspect. + expected: A list of tuples containing the expected items. + """ + qtmodeltester.data_display_may_return_none = True + qtmodeltester.check(cat) + assert cat.rowCount() == len(expected) + for row, items in enumerate(expected): + for col, item in enumerate(items): + assert cat.data(cat.index(row, col)) == item + return validate diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py index 61dcefd66..47d254f86 100644 --- a/tests/helpers/stubs.py +++ b/tests/helpers/stubs.py @@ -33,7 +33,6 @@ from qutebrowser.browser import browsertab from qutebrowser.config import configexc from qutebrowser.utils import usertypes, utils from qutebrowser.mainwindow import mainwindow -from qutebrowser.misc import sql class FakeNetworkCache(QAbstractNetworkCache): @@ -523,37 +522,6 @@ class QuickmarkManagerStub(UrlMarkManagerStub): self.delete(key) -class WebHistoryStub(sql.SqlTable): - - """Stub for the web-history object.""" - - def __init__(self): - super().__init__("History", ['url', 'title', 'atime', 'redirect']) - self.completion = sql.SqlTable("CompletionHistory", - ['url', 'title', 'last_atime']) - - def __contains__(self, url): - q = self.contains_query('url') - return q.run(val=url).value() - - def add_url(self, url, title="", *, redirect=False, atime=None): - self.insert({'url': url, 'title': title, 'atime': atime, - 'redirect': redirect}) - if not redirect: - self.completion.insert({'url': url, - 'title': title, - 'last_atime': atime}) - - def delete_url(self, url): - """Remove all history entries with the given url. - - Args: - url: URL string to delete. - """ - self.delete('url', url) - self.completion.delete('url', url) - - class HostBlockerStub: """Stub for the host-blocker object.""" diff --git a/tests/helpers/utils.py b/tests/helpers/utils.py index 051b7bd50..7dbd7dd25 100644 --- a/tests/helpers/utils.py +++ b/tests/helpers/utils.py @@ -170,16 +170,3 @@ def abs_datapath(): """Get the absolute path to the end2end data directory.""" file_abs = os.path.abspath(os.path.dirname(__file__)) return os.path.join(file_abs, '..', 'end2end', 'data') - - -def validate_model(cat, expected): - """Check that a category contains the expected items in the given order. - - Args: - cat: The category to inspect. - expected: A list of tuples containing the expected items. - """ - assert cat.rowCount() == len(expected) - for row, items in enumerate(expected): - for col, item in enumerate(items): - assert cat.data(cat.index(row, col)) == item diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index 9496accd2..911184dc0 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -21,6 +21,7 @@ import logging import os +import unittest.mock import pytest from PyQt5.QtCore import QUrl @@ -114,7 +115,7 @@ def test_clear(qtbot, tmpdir, hist, mocker): hist.add_url(QUrl('http://www.qutebrowser.org/')) m = mocker.patch('qutebrowser.browser.history.message.confirm_async', - spec=[]) + new=unittest.mock.Mock, spec=[]) hist.clear() assert m.called @@ -134,16 +135,22 @@ def test_delete_url(hist): before = set(hist) hist.delete_url(QUrl('http://example.com/1')) diff = before.difference(set(hist)) - assert diff == set([('http://example.com/1', '', 0, False)]) + assert diff == {('http://example.com/1', '', 0, False)} -@pytest.mark.parametrize('url, atime, title, redirect', [ - ('http://www.example.com', 12346, 'the title', False), - ('http://www.example.com', 12346, 'the title', True) +@pytest.mark.parametrize('url, atime, title, redirect, expected_url', [ + ('http://www.example.com', 12346, 'the title', False, + 'http://www.example.com'), + ('http://www.example.com', 12346, 'the title', True, + 'http://www.example.com'), + ('http://www.example.com/spa ce', 12346, 'the title', False, + 'http://www.example.com/spa%20ce'), + ('https://user:pass@example.com', 12346, 'the title', False, + 'https://user@example.com'), ]) -def test_add_item(qtbot, hist, url, atime, title, redirect): +def test_add_item(qtbot, hist, url, atime, title, redirect, expected_url): hist.add_url(QUrl(url), atime=atime, title=title, redirect=redirect) - assert list(hist) == [(url, title, atime, redirect)] + assert list(hist) == [(expected_url, title, atime, redirect)] def test_add_item_invalid(qtbot, hist, caplog): @@ -164,7 +171,7 @@ def test_add_item_invalid(qtbot, hist, caplog): def test_add_from_tab(hist, level, url, req_url, expected, mock_time, caplog): with caplog.at_level(level): hist.add_from_tab(QUrl(url), QUrl(req_url), 'title') - assert set(list(hist)) == set(expected) + assert set(hist) == set(expected) @pytest.fixture @@ -330,11 +337,3 @@ def test_debug_dump_history_nonexistent(hist, tmpdir): histfile = tmpdir / 'nonexistent' / 'history' with pytest.raises(cmdexc.CommandError): hist.debug_dump_history(str(histfile)) - - -def test_debug_dump_history_oserror(hist, tmpdir): - histfile = tmpdir / 'history' - histfile.write('') - os.chmod(str(histfile), 0) - with pytest.raises(cmdexc.CommandError): - hist.debug_dump_history(str(histfile)) diff --git a/tests/unit/completion/test_completionmodel.py b/tests/unit/completion/test_completionmodel.py index 2c0af3a4b..ce0fe3765 100644 --- a/tests/unit/completion/test_completionmodel.py +++ b/tests/unit/completion/test_completionmodel.py @@ -39,18 +39,18 @@ def test_first_last_item(counts): cat = mock.Mock(spec=['layoutChanged']) cat.rowCount = mock.Mock(return_value=c, spec=[]) model.add_category(cat) - nonempty = [i for i, rowCount in enumerate(counts) if rowCount > 0] - if not nonempty: + data = [i for i, rowCount in enumerate(counts) if rowCount > 0] + if not data: # with no items, first and last should be an invalid index assert not model.first_item().isValid() assert not model.last_item().isValid() else: - first = nonempty[0] - last = nonempty[-1] - # first item of the first nonempty category + first = data[0] + last = data[-1] + # first item of the first data category assert model.first_item().row() == 0 assert model.first_item().parent().row() == first - # last item of the last nonempty category + # last item of the last data category assert model.last_item().row() == counts[last] - 1 assert model.last_item().parent().row() == last diff --git a/tests/unit/completion/test_listcategory.py b/tests/unit/completion/test_listcategory.py index 5b6a5eea4..06c7ac1a6 100644 --- a/tests/unit/completion/test_listcategory.py +++ b/tests/unit/completion/test_listcategory.py @@ -19,10 +19,12 @@ """Tests for CompletionFilterModel.""" +from unittest import mock + import pytest -from helpers import utils from qutebrowser.completion.models import listcategory +from qutebrowser.commands import cmdexc @pytest.mark.parametrize('pattern, before, after', [ @@ -43,8 +45,26 @@ from qutebrowser.completion.models import listcategory [('foo', 'bar'), ('bar', 'foo'), ('bar', 'bar')], [('foo', 'bar'), ('bar', 'foo')]), ]) -def test_set_pattern(pattern, before, after): +def test_set_pattern(pattern, before, after, validate_model): """Validate the filtering and sorting results of set_pattern.""" cat = listcategory.ListCategory('Foo', before) cat.set_pattern(pattern) - utils.validate_model(cat, after) + validate_model(cat, after) + + +def test_delete_cur_item(validate_model): + func = mock.Mock(spec=[]) + cat = listcategory.ListCategory('Foo', [('a', 'b'), ('c', 'd')], + delete_func=func) + idx = cat.index(0, 0) + cat.delete_cur_item(idx) + func.assert_called_once_with(['a', 'b']) + validate_model(cat, [('c', 'd')]) + + +def test_delete_cur_item_no_func(validate_model): + cat = listcategory.ListCategory('Foo', [('a', 'b'), ('c', 'd')]) + idx = cat.index(0, 0) + with pytest.raises(cmdexc.CommandError, match="Cannot delete this item."): + cat.delete_cur_item(idx) + validate_model(cat, [('a', 'b'), ('c', 'd')]) diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index b7b688a81..4fab0c402 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -27,7 +27,8 @@ from PyQt5.QtCore import QUrl from qutebrowser.completion.models import miscmodels, urlmodel, configmodel from qutebrowser.config import sections, value -from qutebrowser.misc import sql +from qutebrowser.utils import objreg +from qutebrowser.browser import history def _check_completions(model, expected): @@ -136,29 +137,33 @@ def bookmarks(bookmark_manager_stub): @pytest.fixture -def history_completion_table(init_sql): - return sql.SqlTable("CompletionHistory", ['url', 'title', 'last_atime']) +def web_history(init_sql, stubs): + """Fixture which provides a web-history object.""" + stub = history.WebHistory() + objreg.register('web-history', stub) + yield stub + objreg.delete('web-history') @pytest.fixture -def web_history(web_history_stub): +def web_history_populated(web_history): """Pre-populate the web-history database.""" - web_history_stub.add_url( - url='http://qutebrowser.org', + web_history.add_url( + url=QUrl('http://qutebrowser.org'), title='qutebrowser', atime=datetime(2015, 9, 5).timestamp() ) - web_history_stub.add_url( - url='https://python.org', + web_history.add_url( + url=QUrl('https://python.org'), title='Welcome to Python.org', atime=datetime(2016, 3, 8).timestamp() ) - web_history_stub.add_url( - url='https://github.com', + web_history.add_url( + url=QUrl('https://github.com'), title='https://github.com', atime=datetime(2016, 5, 1).timestamp() ) - return web_history_stub + return web_history def test_command_completion(qtmodeltester, monkeypatch, stubs, config_stub, @@ -261,8 +266,8 @@ def test_bookmark_completion(qtmodeltester, bookmarks): }) -def test_url_completion(qtmodeltester, config_stub, web_history, quickmarks, - bookmarks): +def test_url_completion(qtmodeltester, config_stub, web_history_populated, + quickmarks, bookmarks): """Test the results of url completion. Verify that: @@ -313,12 +318,12 @@ def test_url_completion(qtmodeltester, config_stub, web_history, quickmarks, ('foo%bar', '', '%', 1), ('foobar', '', '%', 0), ]) -def test_url_completion_pattern(config_stub, web_history_stub, +def test_url_completion_pattern(config_stub, web_history, quickmark_manager_stub, bookmark_manager_stub, url, title, pattern, rowcount): """Test that url completion filters by url and title.""" config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'} - web_history_stub.add_url(url, title) + web_history.add_url(QUrl(url), title) model = urlmodel.url() model.set_pattern(pattern) # 2, 0 is History @@ -349,7 +354,7 @@ def test_url_completion_delete_bookmark(qtmodeltester, config_stub, bookmarks, def test_url_completion_delete_quickmark(qtmodeltester, config_stub, - web_history, quickmarks, bookmarks, + quickmarks, web_history, bookmarks, qtbot): """Test deleting a bookmark from the url completion model.""" config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'} @@ -373,7 +378,7 @@ def test_url_completion_delete_quickmark(qtmodeltester, config_stub, def test_url_completion_delete_history(qtmodeltester, config_stub, - web_history_stub, web_history, + web_history_populated, quickmarks, bookmarks): """Test deleting a history entry.""" config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'} @@ -389,9 +394,9 @@ def test_url_completion_delete_history(qtmodeltester, config_stub, assert model.data(parent) == "History" assert model.data(idx) == 'https://python.org' - assert 'https://python.org' in web_history_stub + assert 'https://python.org' in web_history_populated model.delete_cur_item(idx) - assert 'https://python.org' not in web_history_stub + assert 'https://python.org' not in web_history_populated def test_session_completion(qtmodeltester, session_manager_stub): @@ -504,6 +509,12 @@ def test_setting_option_completion(qtmodeltester, monkeypatch, stubs, }) +def test_setting_option_completion_empty(monkeypatch, stubs, config_stub): + module = 'qutebrowser.completion.models.configmodel' + _patch_configdata(monkeypatch, stubs, module + '.configdata.DATA') + assert configmodel.option('typo') is None + + def test_setting_option_completion_valuelist(qtmodeltester, monkeypatch, stubs, config_stub): module = 'qutebrowser.completion.models.configmodel' @@ -545,6 +556,13 @@ def test_setting_value_completion(qtmodeltester, monkeypatch, stubs, }) +def test_setting_value_completion_empty(monkeypatch, stubs, config_stub): + module = 'qutebrowser.completion.models.configmodel' + _patch_configdata(monkeypatch, stubs, module + '.configdata.DATA') + config_stub.data = {'general': {}} + assert configmodel.value('general', 'typo') is None + + def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub, key_config_stub): """Test the results of keybinding command completion. @@ -583,7 +601,7 @@ def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub, def test_url_completion_benchmark(benchmark, config_stub, quickmark_manager_stub, bookmark_manager_stub, - web_history_stub): + web_history): """Benchmark url completion.""" config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d', 'web-history-max-items': 1000} @@ -595,7 +613,7 @@ def test_url_completion_benchmark(benchmark, config_stub, 'title': ['title{}'.format(i) for i in r] } - web_history_stub.completion.insert_batch(entries) + web_history.completion.insert_batch(entries) quickmark_manager_stub.marks = collections.OrderedDict([ ('title{}'.format(i), 'example.com/{}'.format(i)) diff --git a/tests/unit/completion/test_sqlcategory.py b/tests/unit/completion/test_sqlcategory.py index 10d96f571..c20f8f1c5 100644 --- a/tests/unit/completion/test_sqlcategory.py +++ b/tests/unit/completion/test_sqlcategory.py @@ -23,7 +23,6 @@ import unittest.mock import pytest -from helpers import utils from qutebrowser.misc import sql from qutebrowser.completion.models import sqlcategory from qutebrowser.commands import cmdexc @@ -61,14 +60,14 @@ pytestmark = pytest.mark.usefixtures('init_sql') [('B', 'C', 2), ('A', 'F', 0), ('C', 'A', 1)], [('B', 'C', 2), ('C', 'A', 1), ('A', 'F', 0)]), ]) -def test_sorting(sort_by, sort_order, data, expected): +def test_sorting(sort_by, sort_order, data, expected, validate_model): table = sql.SqlTable('Foo', ['a', 'b', 'c']) for row in data: table.insert({'a': row[0], 'b': row[1], 'c': row[2]}) cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], sort_by=sort_by, sort_order=sort_order) cat.set_pattern('') - utils.validate_model(cat, expected) + validate_model(cat, expected) @pytest.mark.parametrize('pattern, filter_cols, before, after', [ @@ -116,7 +115,7 @@ def test_sorting(sort_by, sort_order, data, expected): [("can't touch this", '', ''), ('a', '', '')], [("can't touch this", '', '')]), ]) -def test_set_pattern(pattern, filter_cols, before, after): +def test_set_pattern(pattern, filter_cols, before, after, validate_model): """Validate the filtering and sorting results of set_pattern.""" table = sql.SqlTable('Foo', ['a', 'b', 'c']) for row in before: @@ -124,15 +123,15 @@ def test_set_pattern(pattern, filter_cols, before, after): filter_fields = [['a', 'b', 'c'][i] for i in filter_cols] cat = sqlcategory.SqlCategory('Foo', filter_fields=filter_fields) cat.set_pattern(pattern) - utils.validate_model(cat, after) + validate_model(cat, after) -def test_select(): +def test_select(validate_model): table = sql.SqlTable('Foo', ['a', 'b', 'c']) table.insert({'a': 'foo', 'b': 'bar', 'c': 'baz'}) cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], select='b, c, a') cat.set_pattern('') - utils.validate_model(cat, [('bar', 'baz', 'foo')]) + validate_model(cat, [('bar', 'baz', 'foo')]) def test_delete_cur_item():