From 4a8b23380ce38c532c9c5d6c9a2d8568ecead37b Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 11 Feb 2018 16:19:28 -0500 Subject: [PATCH 1/3] Trigger save on bookmark-add --toggle. The toggle option was failing to fire the changed signal when it removed a bookmark. This means the bookmark file would not be marked as dirty, and would not be saved on exit/autosave (unless another change was made). --- qutebrowser/browser/urlmarks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/urlmarks.py b/qutebrowser/browser/urlmarks.py index d2f563bb6..0a0dfb4f2 100644 --- a/qutebrowser/browser/urlmarks.py +++ b/qutebrowser/browser/urlmarks.py @@ -280,7 +280,7 @@ class BookmarkManager(UrlMarkManager): if urlstr in self.marks: if toggle: - del self.marks[urlstr] + self.delete(urlstr) return False else: raise AlreadyExistsError("Bookmark already exists!") From d0ca54a0cfbaf98518ab156a22da14cf30296df9 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Tue, 6 Feb 2018 11:14:01 -0500 Subject: [PATCH 2/3] Add unit tests for urlmarks. --- tests/unit/browser/urlmarks.py | 127 +++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/unit/browser/urlmarks.py diff --git a/tests/unit/browser/urlmarks.py b/tests/unit/browser/urlmarks.py new file mode 100644 index 000000000..8e17bd326 --- /dev/null +++ b/tests/unit/browser/urlmarks.py @@ -0,0 +1,127 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2018 Ryan Roden-Corrent (rcorre) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""Tests for the global page history.""" + +from unittest import mock +import pytest +from PyQt5.QtCore import QUrl + +from qutebrowser.browser import urlmarks + + +@pytest.fixture +def bm_file(config_tmpdir): + bm_dir = config_tmpdir / 'bookmarks' + bm_dir.mkdir() + bm_file = bm_dir / 'urls' + return bm_file + + +def test_init(bm_file, fake_save_manager): + bm_file.write('\n'.join([ + 'http://example.com Example Site', + 'http://example.com/foo Foo', + 'http://example.com/bar Bar', + 'http://example.com/notitle', + ])) + + bm = urlmarks.BookmarkManager() + fake_save_manager.add_saveable.assert_called_once_with( + 'bookmark-manager', + bm.save, + mock.ANY, # TODO: compare signal argument for equality + filename=str(bm_file), + ) + + assert list(bm.marks.items()) == [ + ('http://example.com', 'Example Site'), + ('http://example.com/foo', 'Foo'), + ('http://example.com/bar', 'Bar'), + ('http://example.com/notitle', ''), + ] + + +def test_add(bm_file, fake_save_manager, qtbot): + bm = urlmarks.BookmarkManager() + + with qtbot.wait_signal(bm.changed): + bm.add(QUrl('http://example.com'), 'Example Site') + assert list(bm.marks.items()) == [ + ('http://example.com', 'Example Site'), + ] + + with qtbot.wait_signal(bm.changed): + bm.add(QUrl('http://example.com/notitle'), '') + assert list(bm.marks.items()) == [ + ('http://example.com', 'Example Site'), + ('http://example.com/notitle', ''), + ] + + +def test_add_toggle(bm_file, fake_save_manager, qtbot): + bm = urlmarks.BookmarkManager() + + with qtbot.wait_signal(bm.changed): + bm.add(QUrl('http://example.com'), '', toggle=True) + assert 'http://example.com' in bm.marks + + with qtbot.wait_signal(bm.changed): + bm.add(QUrl('http://example.com'), '', toggle=True) + assert 'http://example.com' not in bm.marks + + with qtbot.wait_signal(bm.changed): + bm.add(QUrl('http://example.com'), '', toggle=True) + assert 'http://example.com' in bm.marks + + +def test_add_dupe(bm_file, fake_save_manager, qtbot): + bm = urlmarks.BookmarkManager() + + bm.add(QUrl('http://example.com'), '') + with pytest.raises(urlmarks.AlreadyExistsError): + bm.add(QUrl('http://example.com'), '') + + +def test_delete(bm_file, fake_save_manager, qtbot): + bm = urlmarks.BookmarkManager() + + bm.add(QUrl('http://example.com/foo'), 'Foo') + bm.add(QUrl('http://example.com/bar'), 'Bar') + bm.add(QUrl('http://example.com/baz'), 'Baz') + bm.save() + + with qtbot.wait_signal(bm.changed): + bm.delete('http://example.com/bar') + assert list(bm.marks.items()) == [ + ('http://example.com/foo', 'Foo'), + ('http://example.com/baz', 'Baz'), + ] + + +def test_save(bm_file, fake_save_manager, qtbot): + bm = urlmarks.BookmarkManager() + + bm.add(QUrl('http://example.com'), 'Example Site') + bm.add(QUrl('http://example.com/notitle'), '') + bm.save() + assert bm_file.read().splitlines() == [ + 'http://example.com Example Site', + 'http://example.com/notitle ', + ] From 22d7490126a67fb6df5641cecd36799b3a70d140 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 12 Feb 2018 19:25:24 -0500 Subject: [PATCH 3/3] Remove unused import and TODO from urlmarks test. --- tests/unit/browser/urlmarks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/browser/urlmarks.py b/tests/unit/browser/urlmarks.py index 8e17bd326..df7b3286d 100644 --- a/tests/unit/browser/urlmarks.py +++ b/tests/unit/browser/urlmarks.py @@ -19,7 +19,6 @@ """Tests for the global page history.""" -from unittest import mock import pytest from PyQt5.QtCore import QUrl @@ -46,7 +45,7 @@ def test_init(bm_file, fake_save_manager): fake_save_manager.add_saveable.assert_called_once_with( 'bookmark-manager', bm.save, - mock.ANY, # TODO: compare signal argument for equality + bm.changed, filename=str(bm_file), )