From 39e37b043e8ca433b8f3f83a5fd865551e82afbb Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 16:13:29 +0100 Subject: [PATCH 01/48] Add more tests for deactivated cache Getting closer to 100% completion, add tests for missing paths of DiskCache.insert and DiskCache.cacheSize(). --- tests/unit/browser/test_cache.py | 36 +++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 831b232eb..202e7a5cd 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -19,7 +19,7 @@ """Tests for qutebrowser.browser.cache""" -from PyQt5.QtCore import QUrl, QDateTime +from PyQt5.QtCore import QUrl, QDateTime, QDataStream from PyQt5.QtNetwork import QNetworkDiskCache, QNetworkCacheMetaData from qutebrowser.browser import cache @@ -54,6 +54,16 @@ def test_cache_size_leq_max_cache_size(config_stub, tmpdir): assert disk_cache.cacheSize() < limit+100 +def test_cache_size_deactivated(config_stub, tmpdir): + """Confirm that the cache size returns 0 when deactivated.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.cacheSize() == 0 + + def test_cache_deactivated_private_browsing(config_stub, tmpdir): """Test if cache is deactivated in private-browsing mode.""" config_stub.data = { @@ -118,6 +128,30 @@ def test_cache_insert_data(tmpdir): assert disk_cache.data(QUrl(url)).readAll() == content +def test_cache_deactivated_insert_data(config_stub, tmpdir): + """Insert data when cache is deactivated.""" + # First create an activated cache to get a valid QIODevice from it + url = 'http://qutebrowser.org' + disk_cache = QNetworkDiskCache() + disk_cache.setCacheDirectory(str(tmpdir)) + metadata = QNetworkCacheMetaData() + metadata.setUrl(QUrl(url)) + device = disk_cache.prepare(metadata) + assert device is not None + + # Now create the deactivated cache and insert the valid device created + # above (there probably is a better way to get a valid QIODevice...) + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + + deactivated_cache = QNetworkDiskCache() + deactivated_cache.setCacheDirectory(str(tmpdir)) + assert disk_cache.insert(device) is None + + + def test_cache_remove_data(tmpdir): """Test if a previously inserted entry can be removed from the cache.""" url = 'http://qutebrowser.org' From e1446c3448e5c8cbfc287bb07541fa5ae188f589 Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 16:28:17 +0100 Subject: [PATCH 02/48] Add another test for deactivated cache This one is testing the missing path in updateMetaData for a not activated cache. --- tests/unit/browser/test_cache.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 202e7a5cd..2e2772da4 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -160,7 +160,7 @@ def test_cache_remove_data(tmpdir): preload_cache(disk_cache, url) assert disk_cache.cacheSize() > 0 - assert disk_cache.remove(QUrl(url)) + assert disk_cache.remove(QUrl(url)) == True assert disk_cache.cacheSize() == 0 @@ -210,6 +210,21 @@ def test_cache_update_metadata(tmpdir): assert disk_cache.metaData(QUrl(url)) == metadata +def test_cache_deactivated_update_metadata(config_stub, tmpdir): + """Test updating the meta data when cache is not activated.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + url = 'http://qutebrowser.org' + disk_cache = cache.DiskCache(str(tmpdir)) + + metadata = QNetworkCacheMetaData() + metadata.setUrl(QUrl(url)) + assert metadata.isValid() + assert disk_cache.updateMetaData(metadata) is None + + def test_cache_full(config_stub, tmpdir): """Do a sanity test involving everything.""" config_stub.data = { From 3c2bc670ff8c66f1d087b5761399f7a9eb20275f Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 16:31:47 +0100 Subject: [PATCH 03/48] Add test for alternate path of DiskCache.clear() --- tests/unit/browser/test_cache.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 2e2772da4..05ef3db95 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -180,6 +180,16 @@ def test_cache_clear_activated(config_stub, tmpdir): assert disk_cache.cacheSize() == 0 +def test_cache_clear_deactivated(config_stub, tmpdir): + """Test method clear() on deactivated cache.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.clear() is None + + def test_cache_metadata(tmpdir): """Ensure that DiskCache.metaData() returns exactly what was inserted.""" url = 'http://qutebrowser.org' From 35762955cff1c136734dcbc36465565b451319d6 Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 16:57:00 +0100 Subject: [PATCH 04/48] Fix test_cache_remove_data The test was not using qutebrowsers DiskCache class at all but Qts QNetworkDiskCache. As a result the code paths of DiskCache.remove() were never visited. --- tests/unit/browser/test_cache.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 05ef3db95..d35831b49 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -152,11 +152,14 @@ def test_cache_deactivated_insert_data(config_stub, tmpdir): -def test_cache_remove_data(tmpdir): +def test_cache_remove_data(config_stub, tmpdir): """Test if a previously inserted entry can be removed from the cache.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } url = 'http://qutebrowser.org' - disk_cache = QNetworkDiskCache() - disk_cache.setCacheDirectory(str(tmpdir)) + disk_cache = cache.DiskCache(str(tmpdir)) preload_cache(disk_cache, url) assert disk_cache.cacheSize() > 0 From 571d7a680b2de5c1283c2033d9aa088d3d6a01eb Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 17:09:00 +0100 Subject: [PATCH 05/48] Fix all other tests that weren't using DiskCache So yeah, this explains a lot of the missing paths reported by the coverage analysis. --- tests/unit/browser/test_cache.py | 34 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index d35831b49..222f1878e 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -114,12 +114,15 @@ def test_cache_deactivated_remove_data(config_stub, tmpdir): assert disk_cache.remove(url) == False -def test_cache_insert_data(tmpdir): +def test_cache_insert_data(config_stub, tmpdir): """Test if entries inserted into the cache are actually there.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } url = 'http://qutebrowser.org' content = b'foobar' - disk_cache = QNetworkDiskCache() - disk_cache.setCacheDirectory(str(tmpdir)) + disk_cache = cache.DiskCache(str(tmpdir)) assert disk_cache.cacheSize() == 0 preload_cache(disk_cache, url, content) @@ -130,7 +133,7 @@ def test_cache_insert_data(tmpdir): def test_cache_deactivated_insert_data(config_stub, tmpdir): """Insert data when cache is deactivated.""" - # First create an activated cache to get a valid QIODevice from it + # First create QNetworkDiskCache just to get a valid QIODevice from it url = 'http://qutebrowser.org' disk_cache = QNetworkDiskCache() disk_cache.setCacheDirectory(str(tmpdir)) @@ -139,15 +142,14 @@ def test_cache_deactivated_insert_data(config_stub, tmpdir): device = disk_cache.prepare(metadata) assert device is not None - # Now create the deactivated cache and insert the valid device created + # Now create a deactivated DiskCache and insert the valid device created # above (there probably is a better way to get a valid QIODevice...) config_stub.data = { 'storage': {'cache-size': 1024}, 'general': {'private-browsing': True} } - deactivated_cache = QNetworkDiskCache() - deactivated_cache.setCacheDirectory(str(tmpdir)) + deactivated_cache = cache.DiskCache(str(tmpdir)) assert disk_cache.insert(device) is None @@ -193,14 +195,17 @@ def test_cache_clear_deactivated(config_stub, tmpdir): assert disk_cache.clear() is None -def test_cache_metadata(tmpdir): +def test_cache_metadata(config_stub, tmpdir): """Ensure that DiskCache.metaData() returns exactly what was inserted.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } url = 'http://qutebrowser.org' metadata = QNetworkCacheMetaData() metadata.setUrl(QUrl(url)) assert metadata.isValid() - disk_cache = QNetworkDiskCache() - disk_cache.setCacheDirectory(str(tmpdir)) + disk_cache = cache.DiskCache(str(tmpdir)) device = disk_cache.prepare(metadata) device.write(b'foobar') disk_cache.insert(device) @@ -208,11 +213,14 @@ def test_cache_metadata(tmpdir): assert disk_cache.metaData(QUrl(url)) == metadata -def test_cache_update_metadata(tmpdir): +def test_cache_update_metadata(config_stub, tmpdir): """Test updating the meta data for an existing cache entry.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } url = 'http://qutebrowser.org' - disk_cache = QNetworkDiskCache() - disk_cache.setCacheDirectory(str(tmpdir)) + disk_cache = cache.DiskCache(str(tmpdir)) preload_cache(disk_cache, url, b'foo') assert disk_cache.cacheSize() > 0 From 404da750c6afc95789d2ca0888f9c74301a7791a Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 18:32:14 +0100 Subject: [PATCH 06/48] Fix insertion into wrong cache --- tests/unit/browser/test_cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 222f1878e..6fbf91919 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -19,7 +19,7 @@ """Tests for qutebrowser.browser.cache""" -from PyQt5.QtCore import QUrl, QDateTime, QDataStream +from PyQt5.QtCore import QUrl, QDateTime from PyQt5.QtNetwork import QNetworkDiskCache, QNetworkCacheMetaData from qutebrowser.browser import cache @@ -150,7 +150,7 @@ def test_cache_deactivated_insert_data(config_stub, tmpdir): } deactivated_cache = cache.DiskCache(str(tmpdir)) - assert disk_cache.insert(device) is None + assert deactivated_cache.insert(device) is None From b94f7c7681ef5cd85f78a0e1c6c5b38857187d40 Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 18:49:58 +0100 Subject: [PATCH 07/48] Add test for metaData() with cache deactivated --- tests/unit/browser/test_cache.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 6fbf91919..bc669d637 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -213,6 +213,18 @@ def test_cache_metadata(config_stub, tmpdir): assert disk_cache.metaData(QUrl(url)) == metadata +def test_cache_deactivated_metadata(config_stub, tmpdir): + """Test querying metaData() on not activated cache.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + url = 'http://qutebrowser.org' + + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.metaData(QUrl(url)) == QNetworkCacheMetaData() + + def test_cache_update_metadata(config_stub, tmpdir): """Test updating the meta data for an existing cache entry.""" config_stub.data = { From d127469d78234e7ce0527818945e77593601fa6d Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sat, 31 Oct 2015 21:56:15 +0100 Subject: [PATCH 08/48] Add tests for fileMetaData() One of those three tests is not complete as I couldn't yet find a way to predict the path and name of cached files when using tmpdir. --- tests/unit/browser/test_cache.py | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index bc669d637..e30accd84 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -64,6 +64,54 @@ def test_cache_size_deactivated(config_stub, tmpdir): assert disk_cache.cacheSize() == 0 +def test_cache_existing_metadata_file(config_stub, tmpdir): + """Test querying existing meta data file from activated cache.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } + url = 'http://qutebrowser.org' + content = b'foobar' + + metadata = QNetworkCacheMetaData() + metadata.setUrl(QUrl(url)) + assert metadata.isValid() + + disk_cache = cache.DiskCache(str(tmpdir)) + device = disk_cache.prepare(metadata) + assert device is not None + device.write(content) + disk_cache.insert(device) + disk_cache.updateMetaData(metadata) + + # TODO: Get path and name of file that device has been writing to + #fname = str(tmpdir) + "/cache_" + url + ".cache" + #assert disk_cache.fileMetaData(fname) == metadata + assert True + + +def test_cache_nonexistent_metadata_file(config_stub, tmpdir): + """Test querying nonexistent meta data file from activated cache.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } + + disk_cache = cache.DiskCache(str(tmpdir)) + cache_file = disk_cache.fileMetaData("nosuchfile") + assert cache_file.isValid() == False + + +def test_cache_deactivated_metadata_file(config_stub, tmpdir): + """Test querying meta data file when cache is deactivated.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.fileMetaData("foo") == QNetworkCacheMetaData() + + def test_cache_deactivated_private_browsing(config_stub, tmpdir): """Test if cache is deactivated in private-browsing mode.""" config_stub.data = { From 70a6fe1561aa5a56773c0837687e6cee110e066a Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sun, 1 Nov 2015 14:39:43 +0100 Subject: [PATCH 09/48] Add tests triggering pyqtslot on_config_changed Both settings relevant to the cache (cache-size and private-browsing) are changed, a signal is emitted and the effect on the cache is verified. --- tests/unit/browser/test_cache.py | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index e30accd84..a37169c4d 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -35,6 +35,39 @@ def preload_cache(cache, url='http://www.example.com/', content=b'foobar'): cache.insert(device) +def test_cache_config_change_cache_size(config_stub, tmpdir): + """Change cache size and emit signal to trigger on_config_changed.""" + max_cache_size = 1024 + config_stub.data = { + 'storage': {'cache-size': max_cache_size}, + 'general': {'private-browsing': False} + } + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.maximumCacheSize() == max_cache_size + + config_stub.data['storage']['cache-size'] = max_cache_size * 2 + config_stub.changed.emit('storage', 'cache-size') + assert disk_cache.maximumCacheSize() == max_cache_size * 2 + + +def test_cache_config_change_private_browsing(config_stub, tmpdir): + """Change private-browsing config and emit signal.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False} + } + url = 'http://qutebrowser.org' + content = b'cute' + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.cacheSize() == 0 + preload_cache(disk_cache, url, content) + assert disk_cache.cacheSize() > 0 + + config_stub.data['general']['private-browsing'] = True + config_stub.changed.emit('general', 'private-browsing') + assert disk_cache.cacheSize() == 0 + + def test_cache_size_leq_max_cache_size(config_stub, tmpdir): """Test cacheSize <= MaximumCacheSize when cache is activated.""" limit = 100 From e86795f64450c825418f58cba49236db3aee18fd Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sun, 1 Nov 2015 14:55:03 +0100 Subject: [PATCH 10/48] Add another test for pyqtslot on_config_changed There are now two tests changing the config for private-browsing, covering both True->False and False->True. --- tests/unit/browser/test_cache.py | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index a37169c4d..6040c434b 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -50,17 +50,15 @@ def test_cache_config_change_cache_size(config_stub, tmpdir): assert disk_cache.maximumCacheSize() == max_cache_size * 2 -def test_cache_config_change_private_browsing(config_stub, tmpdir): - """Change private-browsing config and emit signal.""" +def test_cache_config_enable_private_browsing(config_stub, tmpdir): + """Change private-browsing config to True and emit signal.""" config_stub.data = { 'storage': {'cache-size': 1024}, 'general': {'private-browsing': False} } - url = 'http://qutebrowser.org' - content = b'cute' disk_cache = cache.DiskCache(str(tmpdir)) assert disk_cache.cacheSize() == 0 - preload_cache(disk_cache, url, content) + preload_cache(disk_cache) assert disk_cache.cacheSize() > 0 config_stub.data['general']['private-browsing'] = True @@ -68,6 +66,27 @@ def test_cache_config_change_private_browsing(config_stub, tmpdir): assert disk_cache.cacheSize() == 0 +def test_cache_config_disable_private_browsing(config_stub, tmpdir): + """Change private-browsing config to False and emit signal.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': True} + } + url = 'http://qutebrowser.org' + metadata = QNetworkCacheMetaData() + metadata.setUrl(QUrl(url)) + assert metadata.isValid() + + disk_cache = cache.DiskCache(str(tmpdir)) + assert disk_cache.prepare(metadata) is None + + config_stub.data['general']['private-browsing'] = False + config_stub.changed.emit('general', 'private-browsing') + content = b'cute' + preload_cache(disk_cache, url, content) + assert disk_cache.data(QUrl(url)).readAll() == content + + def test_cache_size_leq_max_cache_size(config_stub, tmpdir): """Test cacheSize <= MaximumCacheSize when cache is activated.""" limit = 100 From 37d37148b78ac273b2352081c2b6ad4876b3f7bc Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sun, 1 Nov 2015 19:49:20 +0100 Subject: [PATCH 11/48] Add pragma comment --- qutebrowser/browser/cache.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/cache.py b/qutebrowser/browser/cache.py index 3d023d635..cb532252b 100644 --- a/qutebrowser/browser/cache.py +++ b/qutebrowser/browser/cache.py @@ -65,7 +65,8 @@ class DiskCache(QNetworkDiskCache): """Update cache size/activated if the config was changed.""" if (section, option) == ('storage', 'cache-size'): self.setMaximumCacheSize(config.get('storage', 'cache-size')) - elif (section, option) == ('general', 'private-browsing'): + elif (section, option) == ('general', # pragma: no branch + 'private-browsing'): self._maybe_activate() def cacheSize(self): From baa3bd18a07945b5aa6f5ea8a278bb4c4f72c335 Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Sun, 1 Nov 2015 20:12:57 +0100 Subject: [PATCH 12/48] Fix indentation --- qutebrowser/browser/cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/cache.py b/qutebrowser/browser/cache.py index cb532252b..c51146a3d 100644 --- a/qutebrowser/browser/cache.py +++ b/qutebrowser/browser/cache.py @@ -65,8 +65,8 @@ class DiskCache(QNetworkDiskCache): """Update cache size/activated if the config was changed.""" if (section, option) == ('storage', 'cache-size'): self.setMaximumCacheSize(config.get('storage', 'cache-size')) - elif (section, option) == ('general', # pragma: no branch - 'private-browsing'): + elif (section, option) == ('general', # pragma: no branch + 'private-browsing'): self._maybe_activate() def cacheSize(self): From cbb9fd203a59e09d8d2f26bb02fc33fc8bb30878 Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Thu, 5 Nov 2015 23:08:38 +0100 Subject: [PATCH 13/48] Fix test of existing metadata files In test_cache_existing_metadata_file() we are now getting the correct path to the metadata files, thus making the test useful. This was the last missing test, the cache is now 100% covered (issue#999). --- tests/unit/browser/test_cache.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index 6040c434b..b93add13d 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -136,10 +136,9 @@ def test_cache_existing_metadata_file(config_stub, tmpdir): disk_cache.insert(device) disk_cache.updateMetaData(metadata) - # TODO: Get path and name of file that device has been writing to - #fname = str(tmpdir) + "/cache_" + url + ".cache" - #assert disk_cache.fileMetaData(fname) == metadata - assert True + files = list(tmpdir.visit(fil=lambda path: path.isfile())) + assert len(files) == 1 + assert disk_cache.fileMetaData(str(files[0])) == metadata def test_cache_nonexistent_metadata_file(config_stub, tmpdir): From 35c36725f237a82e55bf2acd92bd6d78c49aaac6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 18:07:29 +0100 Subject: [PATCH 14/48] Disallow :follow-hint outside of hint mode. Fixes #1105. --- qutebrowser/browser/hints.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 05e21c51c..489978762 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -946,7 +946,8 @@ class HintManager(QObject): elems.label.setInnerXml(string) handler() - @cmdutils.register(instance='hintmanager', scope='tab', hide=True) + @cmdutils.register(instance='hintmanager', scope='tab', hide=True, + modes=[usertypes.KeyMode.hint]) def follow_hint(self, keystring=None): """Follow a hint. From 1aebefca18e3e92c860603f7fba66971d709a70f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 18:07:51 +0100 Subject: [PATCH 15/48] bdd: Make "I run ..." work with PrerequisitesError. --- tests/integration/quteprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/quteprocess.py b/tests/integration/quteprocess.py index ab3726d61..7b9daeaca 100644 --- a/tests/integration/quteprocess.py +++ b/tests/integration/quteprocess.py @@ -177,7 +177,7 @@ class QuteProc(testprocess.Process): ipc.send_to_running_instance(self._ipc_socket, [command], target_arg='') self.wait_for(category='commands', module='command', function='run', - message='Calling *') + message='command called: *') def set_setting(self, sect, opt, value): self.send_cmd(':set "{}" "{}" "{}"'.format(sect, opt, value)) From f3d76b5af69f4da5427165940d2b720b77ebdf17 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 18:16:59 +0100 Subject: [PATCH 16/48] Fix :follow-hint with an invalid keystring. --- qutebrowser/browser/hints.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 489978762..2caa4312a 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -959,6 +959,8 @@ class HintManager(QObject): raise cmdexc.CommandError("No hint to follow") else: keystring = self._context.to_follow + elif keystring not in self._context.elems: + raise cmdexc.CommandError("No hint {}!".format(keystring)) self.fire(keystring, force=True) @pyqtSlot('QSize') From dce44f2dc5e78dfe5ab8e22d94a225d7162d9282 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 18:17:13 +0100 Subject: [PATCH 17/48] bdd: Add some first tests for hints. --- tests/integration/data/hints/link.html | 10 ++++++++++ tests/integration/features/hints.feature | 19 +++++++++++++++++++ tests/integration/features/test_hints.py | 21 +++++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 tests/integration/data/hints/link.html create mode 100644 tests/integration/features/hints.feature create mode 100644 tests/integration/features/test_hints.py diff --git a/tests/integration/data/hints/link.html b/tests/integration/data/hints/link.html new file mode 100644 index 000000000..ec4f9f38c --- /dev/null +++ b/tests/integration/data/hints/link.html @@ -0,0 +1,10 @@ + + + + + A link to use hints on + + + Follow me! + + diff --git a/tests/integration/features/hints.feature b/tests/integration/features/hints.feature new file mode 100644 index 000000000..4f565650e --- /dev/null +++ b/tests/integration/features/hints.feature @@ -0,0 +1,19 @@ +Feature: Using hints + + Scenario: Following a hint. + When I open data/hints/link.html + And I run :hint links normal + And I run :follow-hint a + Then the requests should be: + data/hints/link.html + data/hello.txt + + Scenario: Using :follow-hint outside of hint mode (issue 1105) + When I run :follow-hint + Then the error "follow-hint: This command is only allowed in hint mode." should be shown. + + Scenario: Using :follow-hint with an invalid index. + When I open data/hints/link.html + And I run :hint links normal + And I run :follow-hint xyz + Then the error "No hint xyz!" should be shown. diff --git a/tests/integration/features/test_hints.py b/tests/integration/features/test_hints.py new file mode 100644 index 000000000..dc3905215 --- /dev/null +++ b/tests/integration/features/test_hints.py @@ -0,0 +1,21 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2015 Florian Bruhin (The Compiler) +# +# 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 . + +import pytest_bdd as bdd +bdd.scenarios('hints.feature') From 65648da1addcb3b17f6065b8f8d6d11097b793e9 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 18:22:35 +0100 Subject: [PATCH 18/48] Fix #889 during a webpage shutdown. If we're in the middle of closing a WebPage, the webview will still be registered, but already deleted by Qt - so we get a RuntimeError/TypeError there. --- qutebrowser/browser/network/networkmanager.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/qutebrowser/browser/network/networkmanager.py b/qutebrowser/browser/network/networkmanager.py index 436db914e..66ec7595e 100644 --- a/qutebrowser/browser/network/networkmanager.py +++ b/qutebrowser/browser/network/networkmanager.py @@ -360,17 +360,21 @@ class NetworkManager(QNetworkAccessManager): req.setRawHeader('DNT'.encode('ascii'), dnt) req.setRawHeader('X-Do-Not-Track'.encode('ascii'), dnt) - if self._tab_id is None: - current_url = QUrl() # generic NetworkManager, e.g. for downloads - else: + # There are some scenarios where we can't figure out current_url: + # - There's a generic NetworkManager, e.g. for downloads + # - The download was in a tab which is now closed. + current_url = QUrl() + + if self._tab_id is not None: try: webview = objreg.get('webview', scope='tab', window=self._win_id, tab=self._tab_id) - except KeyError: - # https://github.com/The-Compiler/qutebrowser/issues/889 - current_url = QUrl() - else: current_url = webview.url() + except (KeyError, RuntimeError, TypeError): + # https://github.com/The-Compiler/qutebrowser/issues/889 + # Catching RuntimeError and TypeError because we could be in + # the middle of the webpage shutdown here. + current_url = QUrl() self.set_referer(req, current_url) From 9a2125fc1881c63bdbcfcd56d07d0d858c0870ea Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 18:24:23 +0100 Subject: [PATCH 19/48] Update changelog. --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 901509fb9..4135a89e4 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -82,6 +82,7 @@ Fixed - Fixed a crash when a website presents a very small favicon. - Fixed prompting for download directory when `storage -> prompt-download-directory` was unset. +- Fixed crash when using `:follow-hint outside of hint mode. v0.4.1 ------ From 4f6415631fdd8f443be260f082e65942d26383f1 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 19:34:13 +0100 Subject: [PATCH 20/48] tests: Parse function/line being unset for LogLine. --- tests/integration/quteprocess.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/integration/quteprocess.py b/tests/integration/quteprocess.py index 7b9daeaca..592ef99ce 100644 --- a/tests/integration/quteprocess.py +++ b/tests/integration/quteprocess.py @@ -94,8 +94,18 @@ class LogLine(testprocess.Line): else: self.module = module - self.function = match.group('function') - self.line = int(match.group('line')) + function = match.group('function') + if function == 'none': + self.function = None + else: + self.function = function + + line = int(match.group('line')) + if self.function is None and line == 0: + self.line = None + else: + self.line = line + self.message = match.group('message') self.expected = is_ignored_qt_message(self.message) From d288c9598d53b83708ffa3e4c5284388b570065e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 19:34:34 +0100 Subject: [PATCH 21/48] tests: Add some quteprocess.LogLine tests. --- tests/integration/test_quteprocess.py | 50 +++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/integration/test_quteprocess.py b/tests/integration/test_quteprocess.py index 8fdfe1593..495e7438b 100644 --- a/tests/integration/test_quteprocess.py +++ b/tests/integration/test_quteprocess.py @@ -19,8 +19,14 @@ """Test the quteproc fixture used for tests.""" +import logging +import datetime + import pytest +import quteprocess +from qutebrowser.utils import log + def test_quteproc_error_message(qtbot, quteproc): """Make sure the test fails with an unexpected error message.""" @@ -36,3 +42,47 @@ def test_qt_log_ignore(qtbot, quteproc): """Make sure the test passes when logging a qt_log_ignore message.""" with qtbot.waitSignal(quteproc.got_error, raising=True): quteproc.send_cmd(':message-error "SpellCheck: test"') + + +@pytest.mark.parametrize('data, attrs', [ + ( + # Normal message + '01:02:03 DEBUG init earlyinit:init_log:280 Log initialized.', + { + 'timestamp': datetime.datetime(year=1900, month=1, day=1, + hour=1, minute=2, second=3), + 'loglevel': logging.DEBUG, + 'category': 'init', + 'module': 'earlyinit', + 'function': 'init_log', + 'line': 280, + 'message': 'Log initialized.', + 'expected': False, + } + ), + ( + # VDEBUG + '00:00:00 VDEBUG foo foo:foo:0 test', + {'loglevel': log.VDEBUG_LEVEL} + ), + ( + # Unknown module + '00:00:00 WARNING qt Unknown module:none:0 test', + {'module': None, 'function': None, 'line': None}, + ), + ( + # Expected message + '00:00:00 VDEBUG foo foo:foo:0 SpellCheck: test', + {'expected': True}, + ) +]) +def test_log_line_parse(data, attrs): + line = quteprocess.LogLine(data) + for name, expected in attrs.items(): + actual = getattr(line, name) + assert actual == expected, name + + +def test_log_line_no_match(): + with pytest.raises(quteprocess.NoLineMatch): + quteprocess.LogLine("Hello World!") From 6579866abed780438093ecbd6ab29f45c1fbdc0a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 19:55:05 +0100 Subject: [PATCH 22/48] Quote weird Qt functions for logging. --- qutebrowser/utils/log.py | 4 ++++ tests/integration/quteprocess.py | 6 ++++-- tests/integration/test_quteprocess.py | 26 +++++++++++++++++++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/qutebrowser/utils/log.py b/qutebrowser/utils/log.py index 2a87cab4c..2d615e4b3 100644 --- a/qutebrowser/utils/log.py +++ b/qutebrowser/utils/log.py @@ -320,10 +320,14 @@ def qt_message_handler(msg_type, context, msg): level = logging.DEBUG else: level = qt_to_logging[msg_type] + if context.function is None: func = 'none' + elif ':' in context.function: + func = '"{}"'.format(context.function) else: func = context.function + if context.category is None or context.category == 'default': name = 'qt' else: diff --git a/tests/integration/quteprocess.py b/tests/integration/quteprocess.py index 592ef99ce..8989d3911 100644 --- a/tests/integration/quteprocess.py +++ b/tests/integration/quteprocess.py @@ -68,7 +68,9 @@ class LogLine(testprocess.Line): (?P\d\d:\d\d:\d\d) \ (?PVDEBUG|DEBUG|INFO|WARNING|ERROR) \ +(?P\w+) - \ +(?P(\w+|Unknown\ module)):(?P\w+):(?P\d+) + \ +(?P(\w+|Unknown\ module)): + (?P[^"][^:]*|"[^"]+"): + (?P\d+) \ (?P.+) """, re.VERBOSE) @@ -98,7 +100,7 @@ class LogLine(testprocess.Line): if function == 'none': self.function = None else: - self.function = function + self.function = function.strip('"') line = int(match.group('line')) if self.function is None and line == 0: diff --git a/tests/integration/test_quteprocess.py b/tests/integration/test_quteprocess.py index 495e7438b..a301b5115 100644 --- a/tests/integration/test_quteprocess.py +++ b/tests/integration/test_quteprocess.py @@ -74,7 +74,31 @@ def test_qt_log_ignore(qtbot, quteproc): # Expected message '00:00:00 VDEBUG foo foo:foo:0 SpellCheck: test', {'expected': True}, - ) + ), + ( + # Weird Qt location + '00:00:00 DEBUG qt qnetworkreplyhttpimpl:"void ' + 'QNetworkReplyHttpImplPrivate::error(QNetworkReply::NetworkError, ' + 'const QString&)":1929 QNetworkReplyImplPrivate::error: Internal ' + 'problem, this method must only be called once.', + { + 'module': 'qnetworkreplyhttpimpl', + 'function': 'void QNetworkReplyHttpImplPrivate::error(' + 'QNetworkReply::NetworkError, const QString&)', + 'line': 1929 + } + ), + ( + '00:00:00 WARNING qt qxcbxsettings:"QXcbXSettings::' + 'QXcbXSettings(QXcbScreen*)":233 ' + 'QXcbXSettings::QXcbXSettings(QXcbScreen*) Failed to get selection ' + 'owner for XSETTINGS_S atom ', + { + 'module': 'qxcbxsettings', + 'function': 'QXcbXSettings::QXcbXSettings(QXcbScreen*)', + 'line': 233, + } + ), ]) def test_log_line_parse(data, attrs): line = quteprocess.LogLine(data) From 41f7c11ab5bb3eb2e936b27cf8a6aea8cd8f89e1 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 19:55:32 +0100 Subject: [PATCH 23/48] tests: Ignore XSETTINGS_S atom warning. --- pytest.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/pytest.ini b/pytest.ini index 050246538..60890b520 100644 --- a/pytest.ini +++ b/pytest.ini @@ -35,3 +35,4 @@ qt_log_ignore = ^Type conversion already registered from type .* ^QNetworkReplyImplPrivate::error: Internal problem, this method must only be called once\. ^QWaitCondition: Destroyed while threads are still waiting + ^QXcbXSettings::QXcbXSettings(QXcbScreen\*) Failed to get selection owner for XSETTINGS_S atom From 01625834445c841f3b7c999ceaa18261dd5c671b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 20:07:08 +0100 Subject: [PATCH 24/48] Fix check_coverage.py return value. --- scripts/dev/check_coverage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index 65228a10e..f25b9fdaa 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -248,9 +248,9 @@ def main(): """ utils.change_cwd() if '--check-all' in sys.argv: - main_check_all() + return main_check_all() else: - main_check() + return main_check() if __name__ == '__main__': From 5e38861649c72e6c07160e1cf511efe383e9758b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 22:12:10 +0100 Subject: [PATCH 25/48] Fix warning regex in pytest.ini. --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 60890b520..36bc16868 100644 --- a/pytest.ini +++ b/pytest.ini @@ -35,4 +35,4 @@ qt_log_ignore = ^Type conversion already registered from type .* ^QNetworkReplyImplPrivate::error: Internal problem, this method must only be called once\. ^QWaitCondition: Destroyed while threads are still waiting - ^QXcbXSettings::QXcbXSettings(QXcbScreen\*) Failed to get selection owner for XSETTINGS_S atom + ^QXcbXSettings::QXcbXSettings\(QXcbScreen\*\) Failed to get selection owner for XSETTINGS_S atom From 566f94111c044871989aef903ccfaf53c7dcd2ff Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 22:12:24 +0100 Subject: [PATCH 26/48] Don't warn if element vanished on mouse release. This happens somewhat reliably on Ubuntu Trusty with the hint test, and more reliably on Travis. --- qutebrowser/browser/webview.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/webview.py b/qutebrowser/browser/webview.py index 36a61a7ee..54620a0fd 100644 --- a/qutebrowser/browser/webview.py +++ b/qutebrowser/browser/webview.py @@ -292,7 +292,7 @@ class WebView(QWebView): try: elem = webelem.focus_elem(self.page().currentFrame()) except (webelem.IsNullError, RuntimeError): - log.mouse.warning("Element/page vanished!") + log.mouse.debug("Element/page vanished!") return if elem.is_editable(): log.mouse.debug("Clicked editable element (delayed)!") From 2fc1612bd4c3698edab80115836aa9c23bde3e42 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 9 Nov 2015 22:45:51 +0100 Subject: [PATCH 27/48] Fix removing of automatic downloads w/ -1 timeout. With ui -> remove-finished-downloads set to -1, when a download was started with auto_remove=True (like with :adblock-update), there was a QTimer set up with timeout -1, which causes this instead of doing something sane: WARNING: QTimer::singleShot: Timers cannot have negative timeouts --- qutebrowser/browser/downloads.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 70760acb5..e87b3e3a2 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -793,10 +793,15 @@ class DownloadManager(QAbstractListModel): download = DownloadItem(reply, self._win_id, self) download.cancelled.connect( functools.partial(self.remove_item, download)) + delay = config.get('ui', 'remove-finished-downloads') - if delay > -1 or auto_remove: + if delay > -1: download.finished.connect( functools.partial(self.remove_item_delayed, download, delay)) + elif auto_remove: + download.finished.connect( + functools.partial(self.remove_item, download)) + download.data_changed.connect( functools.partial(self.on_data_changed, download)) download.error.connect(self.on_error) From fe8ddd79c0e7d9453c861956b9668e485b977f9e Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Tue, 10 Nov 2015 03:43:02 +0100 Subject: [PATCH 28/48] Use config_stub.set instead of emitting sginal manually --- tests/unit/browser/test_cache.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index b93add13d..b2593aaef 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -45,8 +45,7 @@ def test_cache_config_change_cache_size(config_stub, tmpdir): disk_cache = cache.DiskCache(str(tmpdir)) assert disk_cache.maximumCacheSize() == max_cache_size - config_stub.data['storage']['cache-size'] = max_cache_size * 2 - config_stub.changed.emit('storage', 'cache-size') + config_stub.set('storage', 'cache-size', max_cache_size * 2) assert disk_cache.maximumCacheSize() == max_cache_size * 2 @@ -61,8 +60,7 @@ def test_cache_config_enable_private_browsing(config_stub, tmpdir): preload_cache(disk_cache) assert disk_cache.cacheSize() > 0 - config_stub.data['general']['private-browsing'] = True - config_stub.changed.emit('general', 'private-browsing') + config_stub.set('general', 'private-browsing', True) assert disk_cache.cacheSize() == 0 @@ -80,8 +78,7 @@ def test_cache_config_disable_private_browsing(config_stub, tmpdir): disk_cache = cache.DiskCache(str(tmpdir)) assert disk_cache.prepare(metadata) is None - config_stub.data['general']['private-browsing'] = False - config_stub.changed.emit('general', 'private-browsing') + config_stub.set('general', 'private-browsing', False) content = b'cute' preload_cache(disk_cache, url, content) assert disk_cache.data(QUrl(url)).readAll() == content From a2a1b77857d8e0d682ae495c7db86b8d30a33738 Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Tue, 10 Nov 2015 03:45:38 +0100 Subject: [PATCH 29/48] Undo unnecessary change --- tests/unit/browser/test_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index b2593aaef..2a269051b 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -261,7 +261,7 @@ def test_cache_remove_data(config_stub, tmpdir): preload_cache(disk_cache, url) assert disk_cache.cacheSize() > 0 - assert disk_cache.remove(QUrl(url)) == True + assert disk_cache.remove(QUrl(url)) assert disk_cache.cacheSize() == 0 From aaf62fc6d03571bfce3bb40775b0f0ebe677c65b Mon Sep 17 00:00:00 2001 From: Lamar Pavel Date: Tue, 10 Nov 2015 03:49:18 +0100 Subject: [PATCH 30/48] Add cache tests to perfectly covered files. --- scripts/dev/check_coverage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index 65228a10e..a7602b145 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -48,6 +48,8 @@ PERFECT_FILES = [ ('tests/unit/commands/test_argparser.py', 'qutebrowser/commands/argparser.py'), + ('tests/unit/browser/test_cache.py', + 'qutebrowser/browser/cache.py'), ('tests/unit/browser/test_cookies.py', 'qutebrowser/browser/cookies.py'), ('tests/unit/browser/test_tabhistory.py', From 3fac74656e8dbd94ebe1c408809ddcc9ee563c74 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 06:33:47 +0100 Subject: [PATCH 31/48] bdd: Add some misc. tests. --- tests/integration/features/misc.feature | 28 +++++++++++++++++++++++++ tests/integration/features/test_misc.py | 21 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tests/integration/features/misc.feature create mode 100644 tests/integration/features/test_misc.py diff --git a/tests/integration/features/misc.feature b/tests/integration/features/misc.feature new file mode 100644 index 000000000..76028ae57 --- /dev/null +++ b/tests/integration/features/misc.feature @@ -0,0 +1,28 @@ +Feature: Various utility commands. + + Scenario: :set-cmd-text and :command-accept + When I run :set-cmd-text :message-info "Hello World" + And I run :command-accept + Then the message "Hello World" should be shown. + + Scenario: :set-cmd-text with two commands + When I run :set-cmd-text :message-info test ;; message-error error + And I run :command-accept + Then the message "test" should be shown. + And the error "error" should be shown. + + Scenario: :set-cmd-text with invalid command + When I run :set-cmd-text foo + Then the error "Invalid command text 'foo'." should be shown. + + Scenario: :message-error + When I run :message-error "Hello World" + Then the error "Hello World" should be shown. + + Scenario: :message-info + When I run :message-info "Hello World" + Then the message "Hello World" should be shown. + + Scenario: :message-warning + When I run :message-warning "Hello World" + Then the warning "Hello World" should be shown. diff --git a/tests/integration/features/test_misc.py b/tests/integration/features/test_misc.py new file mode 100644 index 000000000..f92d3fc47 --- /dev/null +++ b/tests/integration/features/test_misc.py @@ -0,0 +1,21 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2015 Florian Bruhin (The Compiler) +# +# 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 . + +import pytest_bdd as bdd +bdd.scenarios('misc.feature') From 6e7d6fb00e2a1e1b1ea8eea7c432096315335e8e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 06:50:31 +0100 Subject: [PATCH 32/48] tests: Use fnmatch for strings in partial_match. --- tests/helpers/test_helper_utils.py | 2 ++ tests/helpers/utils.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/tests/helpers/test_helper_utils.py b/tests/helpers/test_helper_utils.py index 7893d25ab..9512a964f 100644 --- a/tests/helpers/test_helper_utils.py +++ b/tests/helpers/test_helper_utils.py @@ -29,6 +29,7 @@ from helpers import utils # pylint: disable=import-error ({'a': [1, 2, 3]}, {'a': [1]}), ({'a': [1, 2, 3]}, {'a': [..., 2]}), (1.0, 1.00000001), + ("foobarbaz", "foo*baz"), ]) def test_partial_compare_equal(val1, val2): assert utils.partial_compare(val1, val2) @@ -43,6 +44,7 @@ def test_partial_compare_equal(val1, val2): ([1], {1: 2}), ({1: 1}, {1: [1]}), ({'a': [1, 2, 3]}, {'a': [..., 3]}), + ("foo*baz", "foobarbaz"), ]) def test_partial_compare_not_equal(val1, val2): assert not utils.partial_compare(val1, val2) diff --git a/tests/helpers/utils.py b/tests/helpers/utils.py index 7af219d77..775d79bf4 100644 --- a/tests/helpers/utils.py +++ b/tests/helpers/utils.py @@ -20,6 +20,9 @@ """Partial comparison of dicts/lists.""" +import fnmatch + + def _partial_compare_dict(val1, val2): for key in val2: if key not in val1: @@ -71,6 +74,9 @@ def partial_compare(val1, val2): elif isinstance(val2, float): print("Doing float comparison") equal = abs(val1 - val2) < 0.00001 + elif isinstance(val2, str): + print("Doing string comparison") + equal = fnmatch.fnmatchcase(val1, val2) else: print("Comparing via ==") equal = val1 == val2 From f440953adab761958db0119c7aaaab056ef985c8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 07:50:30 +0100 Subject: [PATCH 33/48] bdd: Add step to start a fresh instance. --- tests/integration/features/conftest.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/integration/features/conftest.py b/tests/integration/features/conftest.py index f6d8e5360..94f7a9367 100644 --- a/tests/integration/features/conftest.py +++ b/tests/integration/features/conftest.py @@ -53,6 +53,13 @@ def run_command_given(quteproc, command): quteproc.send_cmd(command) +@bdd.given("I have a fresh instance") +def fresh_instance(quteproc): + """Restart qutebrowser instance for tests needing a fresh state.""" + quteproc.terminate() + quteproc.start() + + @bdd.when(bdd.parsers.parse("I run {command}")) def run_command_when(quteproc, httpbin, command): command = command.replace('(port)', str(httpbin.port)) From 596ed5f545ecd165a31e3785bfaea2fd7d9451fa Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 07:35:52 +0100 Subject: [PATCH 34/48] bdd: Add some more back/forward tests. --- .../integration/features/backforward.feature | 85 +++++++++++++++++-- 1 file changed, 77 insertions(+), 8 deletions(-) diff --git a/tests/integration/features/backforward.feature b/tests/integration/features/backforward.feature index 7c25e7337..dba29fec2 100644 --- a/tests/integration/features/backforward.feature +++ b/tests/integration/features/backforward.feature @@ -4,6 +4,7 @@ Feature: Going back and forward. Scenario: Going back/forward Given I open data/backforward/1.txt When I open data/backforward/2.txt + And I run :tab-only And I run :back And I wait until data/backforward/1.txt is loaded And I reload @@ -15,13 +16,81 @@ Feature: Going back and forward. data/backforward/2.txt data/backforward/1.txt data/backforward/2.txt + And the session should look like: + windows: + - tabs: + - history: + - url: http://localhost:*/data/backforward/1.txt + - active: true + url: http://localhost:*/data/backforward/2.txt - Scenario: Going back without history - Given I open data/backforward/1.txt - When I run :back - Then the error "At beginning of history." should be shown. + Scenario: Going back in a new tab + Given I open data/backforward/1.txt + When I open data/backforward/2.txt + And I run :tab-only + And I run :back -t + And I wait until data/backforward/1.txt is loaded + Then the session should look like: + windows: + - tabs: + - history: + - url: http://localhost:*/data/backforward/1.txt + - active: true + url: http://localhost:*/data/backforward/2.txt + - active: true + history: + - active: true + url: http://localhost:*/data/backforward/1.txt + - url: http://localhost:*/data/backforward/2.txt - Scenario: Going forward without history - Given I open data/backforward/1.txt - When I run :forward - Then the error "At end of history." should be shown. + Scenario: Going back in a new background tab + Given I open data/backforward/1.txt + When I open data/backforward/2.txt + And I run :tab-only + And I run :back -b + And I wait until data/backforward/1.txt is loaded + Then the session should look like: + windows: + - tabs: + - active: true + history: + - url: http://localhost:*/data/backforward/1.txt + - active: true + url: http://localhost:*/data/backforward/2.txt + - history: + - active: true + url: http://localhost:*/data/backforward/1.txt + - url: http://localhost:*/data/backforward/2.txt + + Scenario: Going back in a new window + Given I have a fresh instance + When I open data/backforward/1.txt + And I open data/backforward/2.txt + And I run :back -w + And I wait until data/backforward/1.txt is loaded + Then the session should look like: + windows: + - tabs: + - active: true + history: + - url: about:blank + - url: http://localhost:*/data/backforward/1.txt + - active: true + url: http://localhost:*/data/backforward/2.txt + - tabs: + - active: true + history: + - url: about:blank + - active: true + url: http://localhost:*/data/backforward/1.txt + - url: http://localhost:*/data/backforward/2.txt + + Scenario: Going back without history + Given I open data/backforward/1.txt + When I run :back + Then the error "At beginning of history." should be shown. + + Scenario: Going forward without history + Given I open data/backforward/1.txt + When I run :forward + Then the error "At end of history." should be shown. From 6b4dbad15b7f7962138fad1d92dd3c21617ebd42 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 08:22:06 +0100 Subject: [PATCH 35/48] bdd: Wait until request is done for hints.feature. --- tests/integration/features/hints.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/features/hints.feature b/tests/integration/features/hints.feature index 4f565650e..5c074ec73 100644 --- a/tests/integration/features/hints.feature +++ b/tests/integration/features/hints.feature @@ -4,6 +4,7 @@ Feature: Using hints When I open data/hints/link.html And I run :hint links normal And I run :follow-hint a + And I wait until data/hello.txt is loaded Then the requests should be: data/hints/link.html data/hello.txt From dc3bfb5eb47942eec8e83f710c4f59fc2278cc1f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 08:47:29 +0100 Subject: [PATCH 36/48] bdd: Print ignored lines in testprocess. --- tests/integration/testprocess.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/testprocess.py b/tests/integration/testprocess.py index 4b7608464..4bd016e45 100644 --- a/tests/integration/testprocess.py +++ b/tests/integration/testprocess.py @@ -136,7 +136,9 @@ class Process(QObject): print("INVALID: {}".format(line)) continue - if parsed is not None: + if parsed is None: + print("IGNORED: {}".format(line)) + else: self._data.append(parsed) self.new_data.emit(parsed) From e1c6cd6c6d07a0713812ee2d768cb44c2092cc54 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 08:55:31 +0100 Subject: [PATCH 37/48] tests: Skip test_file for PyQIODevice on OS X. Those seem to cause a hang on Travis on OS X sometimes. --- tests/unit/utils/test_qtutils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/utils/test_qtutils.py b/tests/unit/utils/test_qtutils.py index 8791b6779..768ba0601 100644 --- a/tests/unit/utils/test_qtutils.py +++ b/tests/unit/utils/test_qtutils.py @@ -520,11 +520,13 @@ def test_unset_organization(qapp, orgname, expected): assert qapp.organizationName() == expected -if test_file is not None: +if test_file is not None and sys.platform != 'darwin': # If we were able to import Python's test_file module, we run some code # here which defines unittest TestCases to run the python tests over # PyQIODevice. + # Those are not run on OS X because that seems to cause a hang sometimes. + @pytest.yield_fixture(scope='session', autouse=True) def clean_up_python_testfile(): """Clean up the python testfile after tests if tests didn't.""" From ada4b669bc68ff764df3345904dca87411fbeaa3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 09:23:18 +0100 Subject: [PATCH 38/48] tests: Strip [2s ago] markers from log messages. --- tests/integration/quteprocess.py | 7 +++++-- tests/integration/test_quteprocess.py | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/integration/quteprocess.py b/tests/integration/quteprocess.py index 8989d3911..ac92d1f00 100644 --- a/tests/integration/quteprocess.py +++ b/tests/integration/quteprocess.py @@ -69,7 +69,7 @@ class LogLine(testprocess.Line): \ (?PVDEBUG|DEBUG|INFO|WARNING|ERROR) \ +(?P\w+) \ +(?P(\w+|Unknown\ module)): - (?P[^"][^:]*|"[^"]+"): + (?P[^"][^:]*|"[^"]+"): (?P\d+) \ (?P.+) """, re.VERBOSE) @@ -108,7 +108,10 @@ class LogLine(testprocess.Line): else: self.line = line - self.message = match.group('message') + msg_match = re.match(r'^(\[(?P\d+s ago)\] )?(?P.*)', + match.group('message')) + self.prefix = msg_match.group('prefix') + self.message = msg_match.group('message') self.expected = is_ignored_qt_message(self.message) diff --git a/tests/integration/test_quteprocess.py b/tests/integration/test_quteprocess.py index a301b5115..ad2f3da97 100644 --- a/tests/integration/test_quteprocess.py +++ b/tests/integration/test_quteprocess.py @@ -99,6 +99,11 @@ def test_qt_log_ignore(qtbot, quteproc): 'line': 233, } ), + ( + # With [2s ago] marker + '00:00:00 DEBUG foo foo:foo:0 [2s ago] test', + {'prefix': '2s ago', 'message': 'test'} + ), ]) def test_log_line_parse(data, attrs): line = quteprocess.LogLine(data) From d71618031dfa00f7adb8f75da673f3f900ba186f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 09:24:47 +0100 Subject: [PATCH 39/48] bdd: Decrease timeouts if not on CI. --- tests/integration/quteprocess.py | 2 +- tests/integration/testprocess.py | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/integration/quteprocess.py b/tests/integration/quteprocess.py index ac92d1f00..622c3511d 100644 --- a/tests/integration/quteprocess.py +++ b/tests/integration/quteprocess.py @@ -212,7 +212,7 @@ class QuteProc(testprocess.Process): message=message) line.expected = True - def wait_for(self, timeout=15000, **kwargs): + def wait_for(self, timeout=None, **kwargs): """Override testprocess.wait_for to check past messages. self._data is cleared after every test to provide at least some diff --git a/tests/integration/testprocess.py b/tests/integration/testprocess.py index 4bd016e45..be098169b 100644 --- a/tests/integration/testprocess.py +++ b/tests/integration/testprocess.py @@ -20,6 +20,7 @@ """Base class for a subprocess run for tests..""" import re +import os import time import fnmatch @@ -215,7 +216,7 @@ class Process(QObject): else: return value == expected - def wait_for(self, timeout=15000, **kwargs): + def wait_for(self, timeout=None, **kwargs): """Wait until a given value is found in the data. Keyword arguments to this function get interpreted as attributes of the @@ -225,6 +226,11 @@ class Process(QObject): Return: The matched line. """ + if timeout is None: + if 'CI' in os.environ: + timeout = 15000 + else: + timeout = 5000 # Search existing messages for line in self._data: matches = [] From 374b448e5162c2180599cba3412f200091e8b382 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 18:44:42 +0100 Subject: [PATCH 40/48] Get rid of unnecessary file.readline() calls. --- qutebrowser/misc/editor.py | 2 +- qutebrowser/misc/lineparser.py | 4 ++-- qutebrowser/utils/version.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 4211fe184..9cedf6250 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -82,7 +82,7 @@ class ExternalEditor(QObject): encoding = config.get('general', 'editor-encoding') try: with open(self._filename, 'r', encoding=encoding) as f: - text = ''.join(f.readlines()) # pragma: no branch + text = f.read() # pragma: no branch except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. diff --git a/qutebrowser/misc/lineparser.py b/qutebrowser/misc/lineparser.py index 4a5e1f3ff..8e6cb4aba 100644 --- a/qutebrowser/misc/lineparser.py +++ b/qutebrowser/misc/lineparser.py @@ -213,9 +213,9 @@ class LineParser(BaseLineParser): """Read the data from self._configfile.""" with self._open('r') as f: if self._binary: - self.data = [line.rstrip(b'\n') for line in f.readlines()] + self.data = [line.rstrip(b'\n') for line in f] else: - self.data = [line.rstrip('\n') for line in f.readlines()] + self.data = [line.rstrip('\n') for line in f] def save(self): """Save the config file.""" diff --git a/qutebrowser/utils/version.py b/qutebrowser/utils/version.py index 6975ba5fa..cf1112689 100644 --- a/qutebrowser/utils/version.py +++ b/qutebrowser/utils/version.py @@ -112,7 +112,7 @@ def _release_info(): for fn in glob.glob("/etc/*-release"): try: with open(fn, 'r', encoding='utf-8') as f: - data.append((fn, ''.join(f.readlines()))) # pragma: no branch + data.append((fn, f.read())) # pragma: no branch except OSError: log.misc.exception("Error while reading {}.".format(fn)) return data From 2ca23d803756217a91579547960373e4803aaed3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 18:57:36 +0100 Subject: [PATCH 41/48] Regenerate authors. --- README.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.asciidoc b/README.asciidoc index 43240d97c..8204391ae 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -137,9 +137,9 @@ Contributors, sorted by the number of commits in descending order: * Florian Bruhin * Antoni Boucher * Bruno Oliveira +* Lamar Pavel * Alexander Cogneau * Martin Tournoij -* Lamar Pavel * Raphael Pierzina * Joel Torstensson * Daniel From cd25a25c9611ecf399c5d111cb9f56391eec88cb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 19:14:55 +0100 Subject: [PATCH 42/48] tox: Update werkzeug to 0.11.1 - Fixed a regression on Python 3 in the debugger. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ad89b853b..4eb7aaff7 100644 --- a/tox.ini +++ b/tox.ini @@ -39,7 +39,7 @@ deps = six==1.10.0 termcolor==1.1.0 vulture==0.8.1 - Werkzeug==0.11 + Werkzeug==0.11.1 wheel==0.26.0 xvfbwrapper==0.2.5 commands = From 7701bf602abaf32f5de0835bbb73b011607a729e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 19:21:54 +0100 Subject: [PATCH 43/48] Add --append argument to :set-cmd-text. --- CHANGELOG.asciidoc | 2 ++ doc/help/commands.asciidoc | 3 ++- qutebrowser/mainwindow/statusbar/command.py | 9 ++++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 4135a89e4..f94dfa4eb 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -48,6 +48,8 @@ Added * `colors -> downloads.bg.system` - New command `:download-retry` to retry a failed download. - New command `:download-clear` which replaces `:download-remove --all`. +- `:set-cmd-text` has a new `--append` argument to append to the current + statusbar text. Changed ~~~~~~~ diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index f47bdabd3..98081479d 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -571,7 +571,7 @@ If the option name ends with '?', the value of the option is shown instead. If t [[set-cmd-text]] === set-cmd-text -Syntax: +:set-cmd-text [*--space*] 'text'+ +Syntax: +:set-cmd-text [*--space*] [*--append*] 'text'+ Preset the statusbar to some text. @@ -580,6 +580,7 @@ Preset the statusbar to some text. ==== optional arguments * +*-s*+, +*--space*+: If given, a space is added to the end. +* +*-a*+, +*--append*+: If given, the text is appended to the current text. ==== note * This command does not split arguments after the last argument and handles quotes literally. diff --git a/qutebrowser/mainwindow/statusbar/command.py b/qutebrowser/mainwindow/statusbar/command.py index 1d8105c2f..0a06aed79 100644 --- a/qutebrowser/mainwindow/statusbar/command.py +++ b/qutebrowser/mainwindow/statusbar/command.py @@ -92,7 +92,7 @@ class Command(misc.MinimalLineEditMixin, misc.CommandLineEdit): @cmdutils.register(instance='status-command', name='set-cmd-text', scope='window', maxsplit=0) - def set_cmd_text_command(self, text, space=False): + def set_cmd_text_command(self, text, space=False, append=False): """Preset the statusbar to some text. // @@ -103,6 +103,7 @@ class Command(misc.MinimalLineEditMixin, misc.CommandLineEdit): Args: text: The commandline to set. space: If given, a space is added to the end. + append: If given, the text is appended to the current text. """ tabbed_browser = objreg.get('tabbed-browser', scope='window', window=self._win_id) @@ -122,8 +123,14 @@ class Command(misc.MinimalLineEditMixin, misc.CommandLineEdit): # I'm not sure what's the best thing to do here # https://github.com/The-Compiler/qutebrowser/issues/123 text = text.replace('{url}', url) + if space: text += ' ' + if append: + if not self.text(): + raise cmdexc.CommandError("No current text!") + text = self.text() + text + if not text or text[0] not in modeparsers.STARTCHARS: raise cmdexc.CommandError( "Invalid command text '{}'.".format(text)) From 128465f12bb55138592571510c1f262e0ca2f5f6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 19:22:13 +0100 Subject: [PATCH 44/48] Add some more tests for :set-cmd-text. --- tests/integration/features/misc.feature | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/integration/features/misc.feature b/tests/integration/features/misc.feature index 76028ae57..ecb15dd40 100644 --- a/tests/integration/features/misc.feature +++ b/tests/integration/features/misc.feature @@ -11,6 +11,22 @@ Feature: Various utility commands. Then the message "test" should be shown. And the error "error" should be shown. + Scenario: :set-cmd-text with URL replacement + When I open data/hello.txt + When I run :set-cmd-text :message-info >{url}< + And I run :command-accept + Then the message ">http://localhost:*/hello.txt<" should be shown. + + Scenario: :set-cmd-text with -s and -a + When I run :set-cmd-text -s :message-info "foo + And I run :set-cmd-text -a bar" + And I run :command-accept + Then the message "foo bar" should be shown. + + Scenario: :set-cmd-text with -a but without text + When I run :set-cmd-text -a foo + Then the error "No current text!" should be shown. + Scenario: :set-cmd-text with invalid command When I run :set-cmd-text foo Then the error "Invalid command text 'foo'." should be shown. From a26e99f00491b06c919b9f9465044134981dd9b6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 21:07:49 +0100 Subject: [PATCH 45/48] bdd: Add some tests for :set. --- tests/integration/features/set.feature | 60 ++++++++++++++++++++++++++ tests/integration/features/test_set.py | 32 ++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 tests/integration/features/set.feature create mode 100644 tests/integration/features/test_set.py diff --git a/tests/integration/features/set.feature b/tests/integration/features/set.feature new file mode 100644 index 000000000..1c9916e99 --- /dev/null +++ b/tests/integration/features/set.feature @@ -0,0 +1,60 @@ +Feature: Setting settings. + + Background: + Given I set ui -> message-timeout to 100 + + Scenario: Using :set + When I run :set colors statusbar.bg magenta + Then colors -> statusbar.bg should be magenta + + Scenario: Only a section + When I run :set colors + Then the error "set: Either both section and option have to be given, or neither!" should be shown. + + Scenario: Without value + When I run :set colors statusbar.bg + Then the error "set: The following arguments are required: value" should be shown. + + Scenario: Invalid section + When I run :set blah blub foo + Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown. + + Scenario: Invalid option + When I run :set general blub foo + Then the error "set: NoOptionError - No option 'blub' in section 'general'" should be shown. + + Scenario: Toggling an option + When I run :set general auto-save-config false + And I run :set general auto-save-config! + Then general -> auto-save-config should be True + + Scenario: Toggling a non-bool option + When I run :set colors statusbar.bg! + Then the error "set: Attempted inversion of non-boolean value." should be shown. + + Scenario: Getting an option + When I run :set colors statusbar.bg magenta + And I run :set colors statusbar.bg? + Then the message "colors statusbar.bg = magenta" should be shown. + + Scenario: Using -p + When I run :set -p colors statusbar.bg red + Then the message "colors statusbar.bg = red" should be shown. + + Scenario: Setting a temporary option + # We don't actually check if the option is temporary as this isn't easy + # to check. + When I run :set -t colors statusbar.bg green + Then colors -> statusbar.bg should be green + + Scenario: Opening qute:settings + When I run :set + And I wait for "load status for : LoadStatus.success" in the log + Then the session should look like: + windows: + - tabs: + - active: true + history: + - url: about:blank + - active: true + url: qute:settings diff --git a/tests/integration/features/test_set.py b/tests/integration/features/test_set.py new file mode 100644 index 000000000..fdf767640 --- /dev/null +++ b/tests/integration/features/test_set.py @@ -0,0 +1,32 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2015 Florian Bruhin (The Compiler) +# +# 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 . + +import logging + +import pytest_bdd as bdd +bdd.scenarios('set.feature') + + +@bdd.then(bdd.parsers.parse("{section} -> {option} should be {value}")) +def check_option(quteproc, section, option, value): + quteproc.send_cmd(':set {} {}?'.format(section, option)) + msg = quteproc.wait_for(loglevel=logging.INFO, category='message', + message='{} {} = *'.format(section, option)) + actual_value = msg.message.split(' = ')[1] + assert actual_value == value From 54e2cea460915102358733aee3ac1f5944dc153a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 21:27:42 +0100 Subject: [PATCH 46/48] Fix some corner cases with :set. --- CHANGELOG.asciidoc | 3 ++- qutebrowser/config/config.py | 10 ++++--- tests/integration/features/set.feature | 37 ++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f94dfa4eb..ddfbc0116 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -84,7 +84,8 @@ Fixed - Fixed a crash when a website presents a very small favicon. - Fixed prompting for download directory when `storage -> prompt-download-directory` was unset. -- Fixed crash when using `:follow-hint outside of hint mode. +- Fixed crash when using `:follow-hint` outside of hint mode. +- Fixed crash when using `:set foo bar?` with invalid section/option. v0.4.1 ------ diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 55502c5da..7f1727924 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -699,12 +699,12 @@ class ConfigManager(QObject): tabbed_browser.openurl(QUrl('qute:settings'), newtab=False) return - if option.endswith('?'): + if option.endswith('?') and option != '?': option = option[:-1] print_ = True else: try: - if option.endswith('!') and value is None: + if option.endswith('!') and option != '!' and value is None: option = option[:-1] val = self.get(section_, option) layer = 'temp' if temp else 'conf' @@ -724,7 +724,11 @@ class ConfigManager(QObject): e.__class__.__name__, e)) if print_: - val = self.get(section_, option, transformed=False) + try: + val = self.get(section_, option, transformed=False) + except configexc.Error as e: + raise cmdexc.CommandError("set: {} - {}".format( + e.__class__.__name__, e)) message.info(win_id, "{} {} = {}".format( section_, option, val), immediately=True) diff --git a/tests/integration/features/set.feature b/tests/integration/features/set.feature index 1c9916e99..92c6c07bf 100644 --- a/tests/integration/features/set.feature +++ b/tests/integration/features/set.feature @@ -41,6 +41,11 @@ Feature: Setting settings. When I run :set -p colors statusbar.bg red Then the message "colors statusbar.bg = red" should be shown. + Scenario: Using ! and -p + When I run :set general auto-save-config false + And I run :set -p general auto-save-config! + Then the message "general auto-save-config = True" should be shown. + Scenario: Setting a temporary option # We don't actually check if the option is temporary as this isn't easy # to check. @@ -58,3 +63,35 @@ Feature: Setting settings. - url: about:blank - active: true url: qute:settings + + Scenario: Empty option with ? (issue 1109) + When I run :set general ? + Then the error "set: The following arguments are required: value" should be shown. + + Scenario: Invalid section and empty option with ? (issue 1109) + When I run :set blah ? + Then the error "set: The following arguments are required: value" should be shown. + + Scenario: Invalid option with ? (issue 1109) + When I run :set general foo? + Then the error "set: NoOptionError - No option 'foo' in section 'general'" should be shown. + + Scenario: Invalid section/option with ? (issue 1109) + When I run :set blah foo ? + Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown. + + Scenario: Empty option with ! + When I run :set general ! + Then the error "set: The following arguments are required: value" should be shown. + + Scenario: Invalid section and empty option with ! + When I run :set blah ! + Then the error "set: The following arguments are required: value" should be shown. + + Scenario: Invalid option with ! + When I run :set general foo! + Then the error "set: NoOptionError - No option 'foo' in section 'general'" should be shown. + + Scenario: Invalid section/option with ! + When I run :set blah foo ! + Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown. From d99f9a3a20a7905d49abf9683511f7b862896840 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 10 Nov 2015 22:09:36 +0100 Subject: [PATCH 47/48] Improve :set error messages. --- qutebrowser/config/config.py | 23 +++++++++++++++-------- tests/integration/features/set.feature | 16 ++++++++++------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 7f1727924..09cde457e 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -29,6 +29,7 @@ import sys import os.path import functools import configparser +import contextlib import collections import collections.abc @@ -666,6 +667,18 @@ class ConfigManager(QObject): newval = val.typ.transform(newval) return newval + @contextlib.contextmanager + def _handle_config_error(self): + """Catch errors in set_command and raise CommandError.""" + try: + yield + except (configexc.NoOptionError, configexc.NoSectionError, + configexc.ValidationError) as e: + raise cmdexc.CommandError("set: {}".format(e)) + except (configexc.Error, configparser.Error) as e: + raise cmdexc.CommandError("set: {} - {}".format( + e.__class__.__name__, e)) + @cmdutils.register(name='set', instance='config', win_id='win_id', completion=[Completion.section, Completion.option, Completion.value]) @@ -703,7 +716,7 @@ class ConfigManager(QObject): option = option[:-1] print_ = True else: - try: + with self._handle_config_error(): if option.endswith('!') and option != '!' and value is None: option = option[:-1] val = self.get(section_, option) @@ -719,16 +732,10 @@ class ConfigManager(QObject): else: raise cmdexc.CommandError("set: The following arguments " "are required: value") - except (configexc.Error, configparser.Error) as e: - raise cmdexc.CommandError("set: {} - {}".format( - e.__class__.__name__, e)) if print_: - try: + with self._handle_config_error(): val = self.get(section_, option, transformed=False) - except configexc.Error as e: - raise cmdexc.CommandError("set: {} - {}".format( - e.__class__.__name__, e)) message.info(win_id, "{} {} = {}".format( section_, option, val), immediately=True) diff --git a/tests/integration/features/set.feature b/tests/integration/features/set.feature index 92c6c07bf..a0c3a8c55 100644 --- a/tests/integration/features/set.feature +++ b/tests/integration/features/set.feature @@ -17,11 +17,11 @@ Feature: Setting settings. Scenario: Invalid section When I run :set blah blub foo - Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown. + Then the error "set: Section 'blah' does not exist!" should be shown. Scenario: Invalid option When I run :set general blub foo - Then the error "set: NoOptionError - No option 'blub' in section 'general'" should be shown. + Then the error "set: No option 'blub' in section 'general'" should be shown. Scenario: Toggling an option When I run :set general auto-save-config false @@ -46,6 +46,10 @@ Feature: Setting settings. And I run :set -p general auto-save-config! Then the message "general auto-save-config = True" should be shown. + Scenario: Setting an invalid value + When I run :set general auto-save-config blah + Then the error "set: Invalid value 'blah' - must be a boolean!" should be shown. + Scenario: Setting a temporary option # We don't actually check if the option is temporary as this isn't easy # to check. @@ -74,11 +78,11 @@ Feature: Setting settings. Scenario: Invalid option with ? (issue 1109) When I run :set general foo? - Then the error "set: NoOptionError - No option 'foo' in section 'general'" should be shown. + Then the error "set: No option 'foo' in section 'general'" should be shown. Scenario: Invalid section/option with ? (issue 1109) When I run :set blah foo ? - Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown. + Then the error "set: Section 'blah' does not exist!" should be shown. Scenario: Empty option with ! When I run :set general ! @@ -90,8 +94,8 @@ Feature: Setting settings. Scenario: Invalid option with ! When I run :set general foo! - Then the error "set: NoOptionError - No option 'foo' in section 'general'" should be shown. + Then the error "set: No option 'foo' in section 'general'" should be shown. Scenario: Invalid section/option with ! When I run :set blah foo ! - Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown. + Then the error "set: Section 'blah' does not exist!" should be shown. From 6bd45bbf24ae9b31e5fcc8cb5f969f91c404bfd8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 11 Nov 2015 07:48:36 +0100 Subject: [PATCH 48/48] tests: Add some code for MHTML integration tests. --- .../data/downloads/mhtml/simple/requests | 1 + .../data/downloads/mhtml/simple/simple.html | 10 ++ .../data/downloads/mhtml/simple/simple.mht | 20 ++++ tests/integration/test_mhtml_e2e.py | 103 ++++++++++++++++++ 4 files changed, 134 insertions(+) create mode 100644 tests/integration/data/downloads/mhtml/simple/requests create mode 100644 tests/integration/data/downloads/mhtml/simple/simple.html create mode 100644 tests/integration/data/downloads/mhtml/simple/simple.mht create mode 100644 tests/integration/test_mhtml_e2e.py diff --git a/tests/integration/data/downloads/mhtml/simple/requests b/tests/integration/data/downloads/mhtml/simple/requests new file mode 100644 index 000000000..aff865ec9 --- /dev/null +++ b/tests/integration/data/downloads/mhtml/simple/requests @@ -0,0 +1 @@ +simple.html diff --git a/tests/integration/data/downloads/mhtml/simple/simple.html b/tests/integration/data/downloads/mhtml/simple/simple.html new file mode 100644 index 000000000..7584c3f91 --- /dev/null +++ b/tests/integration/data/downloads/mhtml/simple/simple.html @@ -0,0 +1,10 @@ + + + + + Simple MHTML test + + + normal link to another page + + diff --git a/tests/integration/data/downloads/mhtml/simple/simple.mht b/tests/integration/data/downloads/mhtml/simple/simple.mht new file mode 100644 index 000000000..d0b7a7c48 --- /dev/null +++ b/tests/integration/data/downloads/mhtml/simple/simple.mht @@ -0,0 +1,20 @@ +Content-Type: multipart/related; boundary="---=_qute-6d584056-b1e4-4882-91e6-d4a6d23adb67" +MIME-Version: 1.0 + +-----=_qute-6d584056-b1e4-4882-91e6-d4a6d23adb67 +Content-Location: http://localhost:1234/data/downloads/mhtml/simple/simple.html +MIME-Version: 1.0 +Content-Type: text/html; charset="UTF-8" +Content-Transfer-Encoding: quoted-printable + + +=20=20=20=20=20=20=20=20 +=20=20=20=20=20=20=20=20Simple=20MHTML=20test +=20=20=20=20 +=20=20=20=20 +=20=20=20=20=20=20=20=20normal=20link=20to=20another=20page= + +=20=20=20=20 + + +-----=_qute-6d584056-b1e4-4882-91e6-d4a6d23adb67-- diff --git a/tests/integration/test_mhtml_e2e.py b/tests/integration/test_mhtml_e2e.py new file mode 100644 index 000000000..c3185c007 --- /dev/null +++ b/tests/integration/test_mhtml_e2e.py @@ -0,0 +1,103 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2015 Florian Bruhin (The Compiler) +# +# 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 . + +"""Test mhtml downloads based on sample files.""" + +import os +import re +import os.path + +import pytest + + +def collect_tests(): + basedir = os.path.dirname(__file__) + datadir = os.path.join(basedir, 'data', 'downloads', 'mhtml') + files = os.listdir(datadir) + return files + + +def normalize_line(line): + line = line.rstrip('\n') + line = re.sub('boundary="---=_qute-[0-9a-f-]+"', + 'boundary="---=_qute-UUID"', line) + line = re.sub('^-----=_qute-[0-9a-f-]+$', '-----=_qute-UUID', line) + line = re.sub(r'localhost:\d{1,5}', 'localhost:(port)', line) + return line + + +class DownloadDir: + + """Abstraction over a download directory.""" + + def __init__(self, tmpdir): + self._tmpdir = tmpdir + self.location = str(tmpdir) + + def read_file(self): + files = self._tmpdir.listdir() + assert len(files) == 1 + + with open(str(files[0]), 'r', encoding='utf-8') as f: + return f.readlines() + + def compare_mhtml(self, filename): + with open(filename, 'r', encoding='utf-8') as f: + expected_data = [normalize_line(line) for line in f] + actual_data = self.read_file() + actual_data = [normalize_line(line) for line in actual_data] + assert actual_data == expected_data + + +@pytest.fixture +def download_dir(tmpdir): + return DownloadDir(tmpdir) + + +@pytest.mark.parametrize('test_name', collect_tests()) +def test_mhtml(test_name, download_dir, quteproc, httpbin): + quteproc.set_setting('storage', 'download-directory', + download_dir.location) + quteproc.set_setting('storage', 'prompt-download-directory', 'false') + + test_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), + 'data', 'downloads', 'mhtml', test_name) + test_path = 'data/downloads/mhtml/{}'.format(test_name) + + quteproc.open_path('{}/{}.html'.format(test_path, test_name)) + download_dest = os.path.join(download_dir.location, + '{}-downloaded.mht'.format(test_name)) + quteproc.send_cmd(':download --mhtml --dest "{}"'.format(download_dest)) + quteproc.wait_for(category='downloads', module='mhtml', + function='finish_file', + message='All assets downloaded, ready to finish off!') + + expected_file = os.path.join(test_dir, '{}.mht'.format(test_name)) + download_dir.compare_mhtml(expected_file) + + with open(os.path.join(test_dir, 'requests'), encoding='utf-8') as f: + expected_requests = [] + for line in f: + if line.startswith('#'): + continue + path = '/{}/{}'.format(test_path, line.strip()) + expected_requests.append(httpbin.Request('GET', path)) + + actual_requests = httpbin.get_requests() + assert sorted(actual_requests) == sorted(expected_requests)