From 2d850f710625eacda292ef8fdd7c371fb21916d5 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Thu, 3 Dec 2015 21:12:51 +0100 Subject: [PATCH 01/21] Add tests for first functions of adblock.py --- tests/unit/browser/test_adblock.py | 107 +++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 tests/unit/browser/test_adblock.py diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py new file mode 100644 index 000000000..4130cfc9a --- /dev/null +++ b/tests/unit/browser/test_adblock.py @@ -0,0 +1,107 @@ +import pytest +from qutebrowser.browser import adblock +from qutebrowser.config import config +from qutebrowser.utils import objreg +import os +import zipfile +import io + +# @pytest.yield_fixture +# def default_config(): +# """Fixture that provides and registers an empty default config object.""" +# config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) +# objreg.register('config', config_obj) +# yield config_obj +# objreg.delete('config') + +def create_text_files(files_names, directory): + """Returns a list of created text files""" + directory = str(directory) + created_files = [] + for file_name in files_names: + test_file = os.path.join(directory, file_name) + with open(test_file, 'w') as f : + f.write('inside ' + file_name) + created_files.append(test_file) + return created_files + +def create_zipfile(files_names, directory): + """Returns a zipfile populated with created files and its name""" + directory = str(directory) + files = create_text_files(files_names, directory) + # include created files in a ZipFile + zipfile_name = os.path.join(directory,'test.zip') + with zipfile.ZipFile(zipfile_name, 'w') as zf: + for file_name in files : + zf.write(file_name, arcname=os.path.basename(file_name)) + # Removes path from file name + return zf, zipfile_name + +class TestGuessZipFilename : + """ Test function adblock.guess_zip_filename() """ + + def test_with_single_file(self, tmpdir): + """Zip provided only contains a single file""" + zf = create_zipfile(['testa'], tmpdir)[0] + assert adblock.guess_zip_filename(zf) == 'testa' + # guess_zip_filename doesn't include the root slash / + # whereas os.path.join() does, so we exclude first character + + def test_with_multiple_files(self, tmpdir): + """Zip provided contains multiple files including hosts""" + zf = create_zipfile(['testa','testb','hosts','testc'], tmpdir)[0] + assert adblock.guess_zip_filename(zf) == 'hosts' + # guess_zip_filename doesn't include the root slash / + # whereas os.path.join() does, so we exclude first character + + def test_without_hosts_file(self, tmpdir): + """Zip provided does not contain any hosts file""" + zf = create_zipfile(['testa','testb','testd','testc'], tmpdir)[0] + with pytest.raises(FileNotFoundError): + adblock.guess_zip_filename(zf) + +class TestGetFileObj : + """Test Function adblock.get_fileobj()""" + + def test_with_zipfile(self, tmpdir): + zf_name = create_zipfile(['testa','testb','hosts','testc'], tmpdir)[1] + zipobj = open(zf_name, 'rb') + assert adblock.get_fileobj(zipobj).read() == "inside hosts" + + def test_with_text_file(self, tmpdir): + test_file = open(create_text_files(['testfile'], tmpdir)[0], 'rb') + assert adblock.get_fileobj(test_file).read() == "inside testfile" + +class TestIsWhitelistedHost : + + # def test_with_no_whitelist(self): + # config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) + # objreg.register('config', config_obj) + # assert adblock.is_whitelisted_host('pimpmytest.com') == False + # objreg.delete('config') + + def test_with_no_whitelist(self): + # FIXME Behaves like a mismatch + config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) + default_config.remove_option('content','host-blocking-whitelist') + objreg.register('config', config_obj) + assert adblock.is_whitelisted_host('pimpmytest.com') == False + objreg.delete('config') + + def test_with_match(self): + config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) + config_obj.set('conf','content','host-blocking-whitelist','qutebrowser.org') + objreg.register('config', config_obj) + assert adblock.is_whitelisted_host('qutebrowser.org') == True + objreg.delete('config') + + def test_without_match(self): + config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) + config_obj.set('conf','content','host-blocking-whitelist','cutebrowser.org') + objreg.register('config', config_obj) + assert adblock.is_whitelisted_host('qutebrowser.org') == False + objreg.delete('config') + +class TestHostBlocker : + pass + #testBlocker = adblock.HostBlocker() From 8dd0249af940b496b57352006a6600214805b157 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Thu, 3 Dec 2015 22:00:39 +0100 Subject: [PATCH 02/21] Code cleanup - Pep8 --- tests/unit/browser/test_adblock.py | 83 +++++++++++++++++++----------- 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 4130cfc9a..30ce40b3d 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -1,11 +1,34 @@ +#!/usr/bin/env python3 + +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2014-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 . + +"""Tests for qutebrowser.browser.adblock""" + import pytest from qutebrowser.browser import adblock from qutebrowser.config import config from qutebrowser.utils import objreg import os import zipfile -import io +# TODO Should I use it ? And how ? # @pytest.yield_fixture # def default_config(): # """Fixture that provides and registers an empty default config object.""" @@ -14,57 +37,61 @@ import io # yield config_obj # objreg.delete('config') + def create_text_files(files_names, directory): - """Returns a list of created text files""" + """Returns a list of text files created + with given names in given directory""" directory = str(directory) created_files = [] for file_name in files_names: test_file = os.path.join(directory, file_name) - with open(test_file, 'w') as f : + with open(test_file, 'w') as f: f.write('inside ' + file_name) created_files.append(test_file) return created_files + def create_zipfile(files_names, directory): """Returns a zipfile populated with created files and its name""" directory = str(directory) files = create_text_files(files_names, directory) # include created files in a ZipFile - zipfile_name = os.path.join(directory,'test.zip') + zipfile_name = os.path.join(directory, 'test.zip') with zipfile.ZipFile(zipfile_name, 'w') as zf: - for file_name in files : + for file_name in files: zf.write(file_name, arcname=os.path.basename(file_name)) # Removes path from file name return zf, zipfile_name -class TestGuessZipFilename : + +class TestGuessZipFilename: """ Test function adblock.guess_zip_filename() """ def test_with_single_file(self, tmpdir): """Zip provided only contains a single file""" - zf = create_zipfile(['testa'], tmpdir)[0] - assert adblock.guess_zip_filename(zf) == 'testa' - # guess_zip_filename doesn't include the root slash / - # whereas os.path.join() does, so we exclude first character + zf = create_zipfile(['test_a'], tmpdir)[0] + assert adblock.guess_zip_filename(zf) == 'test_a' def test_with_multiple_files(self, tmpdir): """Zip provided contains multiple files including hosts""" - zf = create_zipfile(['testa','testb','hosts','testc'], tmpdir)[0] + names = ['test_a', 'test_b', 'hosts', 'test_c'] + zf = create_zipfile(names, tmpdir)[0] assert adblock.guess_zip_filename(zf) == 'hosts' - # guess_zip_filename doesn't include the root slash / - # whereas os.path.join() does, so we exclude first character def test_without_hosts_file(self, tmpdir): """Zip provided does not contain any hosts file""" - zf = create_zipfile(['testa','testb','testd','testc'], tmpdir)[0] + names = ['test_a', 'test_b', 'test_d', 'test_c'] + zf = create_zipfile(names, tmpdir)[0] with pytest.raises(FileNotFoundError): adblock.guess_zip_filename(zf) -class TestGetFileObj : + +class TestGetFileObj: """Test Function adblock.get_fileobj()""" def test_with_zipfile(self, tmpdir): - zf_name = create_zipfile(['testa','testb','hosts','testc'], tmpdir)[1] + names = ['test_a', 'test_b', 'hosts', 'test_c'] + zf_name = create_zipfile(names, tmpdir)[1] zipobj = open(zf_name, 'rb') assert adblock.get_fileobj(zipobj).read() == "inside hosts" @@ -72,36 +99,34 @@ class TestGetFileObj : test_file = open(create_text_files(['testfile'], tmpdir)[0], 'rb') assert adblock.get_fileobj(test_file).read() == "inside testfile" -class TestIsWhitelistedHost : +class TestIsWhitelistedHost: + """Test function adblock.is_whitelisted_host""" + + # FIXME Error since we deleted host-blocking-whitelist + # If we don't remove host-block-whitelist, test behaves as in a mismatch # def test_with_no_whitelist(self): # config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) + # config_obj.remove_option('content','host-blocking-whitelist') # objreg.register('config', config_obj) # assert adblock.is_whitelisted_host('pimpmytest.com') == False # objreg.delete('config') - def test_with_no_whitelist(self): - # FIXME Behaves like a mismatch - config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) - default_config.remove_option('content','host-blocking-whitelist') - objreg.register('config', config_obj) - assert adblock.is_whitelisted_host('pimpmytest.com') == False - objreg.delete('config') - def test_with_match(self): config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) - config_obj.set('conf','content','host-blocking-whitelist','qutebrowser.org') + config_obj.set('conf', 'content', 'host-blocking-whitelist', 'qutebrowser.org') objreg.register('config', config_obj) assert adblock.is_whitelisted_host('qutebrowser.org') == True objreg.delete('config') def test_without_match(self): config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) - config_obj.set('conf','content','host-blocking-whitelist','cutebrowser.org') + config_obj.set('conf', 'content', 'host-blocking-whitelist', 'cutebrowser.org') objreg.register('config', config_obj) assert adblock.is_whitelisted_host('qutebrowser.org') == False objreg.delete('config') -class TestHostBlocker : + +class TestHostBlocker: + # TODO pass - #testBlocker = adblock.HostBlocker() From 5369fc30bcd8a31edb072e612f993b9b497739d5 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Thu, 3 Dec 2015 22:13:50 +0100 Subject: [PATCH 03/21] Remove old pull request commits --- qutebrowser/config/config.py | 2 +- qutebrowser/config/sections.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 57837084f..42f9395e2 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -619,7 +619,7 @@ class ConfigManager(QObject): optname = self.optionxform(optname) existed = optname in sectdict if existed: - sectdict.delete(optname) + del sectdict[optname] # WORKAROUND for https://bitbucket.org/logilab/pylint/issues/659/ self.get.cache_clear() # pylint: disable=no-member return existed diff --git a/qutebrowser/config/sections.py b/qutebrowser/config/sections.py index 0253777cc..2a20b5b80 100644 --- a/qutebrowser/config/sections.py +++ b/qutebrowser/config/sections.py @@ -72,10 +72,6 @@ class Section: """Get value keys.""" return self.values.keys() - def delete(self, key): - """Delete item with given key""" - del self.values[key] - def setv(self, layer, key, value, interpolated): """Set the value on a layer. From 95b200ead912b53da6c59d9757358bca1e7bc8f5 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Fri, 4 Dec 2015 01:52:00 +0100 Subject: [PATCH 04/21] Pylint code cleanup --- tests/unit/browser/test_adblock.py | 37 +++++++++++++++++++----------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 30ce40b3d..41f24fdd6 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -1,6 +1,5 @@ -#!/usr/bin/env python3 - # vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: +#!/usr/bin/env python3 # Copyright 2014-2015 Florian Bruhin (The Compiler) # @@ -21,18 +20,22 @@ """Tests for qutebrowser.browser.adblock""" +import os +import zipfile + import pytest + from qutebrowser.browser import adblock from qutebrowser.config import config from qutebrowser.utils import objreg -import os -import zipfile # TODO Should I use it ? And how ? # @pytest.yield_fixture # def default_config(): # """Fixture that provides and registers an empty default config object.""" -# config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) +# config_obj = config.ConfigManager(configdir=None, +# fname=None, +# relaxed=True) # objreg.register('config', config_obj) # yield config_obj # objreg.delete('config') @@ -45,7 +48,7 @@ def create_text_files(files_names, directory): created_files = [] for file_name in files_names: test_file = os.path.join(directory, file_name) - with open(test_file, 'w') as f: + with open(test_file, 'w', encoding='utf-8') as f: f.write('inside ' + file_name) created_files.append(test_file) return created_files @@ -106,24 +109,32 @@ class TestIsWhitelistedHost: # FIXME Error since we deleted host-blocking-whitelist # If we don't remove host-block-whitelist, test behaves as in a mismatch # def test_with_no_whitelist(self): - # config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) + # config_obj = config.ConfigManager(configdir=None, + # fname=None, + # relaxed=True) # config_obj.remove_option('content','host-blocking-whitelist') # objreg.register('config', config_obj) # assert adblock.is_whitelisted_host('pimpmytest.com') == False # objreg.delete('config') def test_with_match(self): - config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) - config_obj.set('conf', 'content', 'host-blocking-whitelist', 'qutebrowser.org') + config_obj = config.ConfigManager(configdir=None, fname=None, + relaxed=True) + config_obj.set_command(0, section_='content', + option='host-blocking-whitelist', + value='qutebrowser.org') objreg.register('config', config_obj) - assert adblock.is_whitelisted_host('qutebrowser.org') == True + assert adblock.is_whitelisted_host('qutebrowser.org') objreg.delete('config') def test_without_match(self): - config_obj = config.ConfigManager(configdir=None, fname=None, relaxed=True) - config_obj.set('conf', 'content', 'host-blocking-whitelist', 'cutebrowser.org') + config_obj = config.ConfigManager(configdir=None, fname=None, + relaxed=True) + config_obj.set_command(0, section_='content', + option='host-blocking-whitelist', + value='cutebrowser.org') objreg.register('config', config_obj) - assert adblock.is_whitelisted_host('qutebrowser.org') == False + assert not adblock.is_whitelisted_host('qutebrowser.org') objreg.delete('config') From 8222a862018e700c0cd880d5263f788a59586b33 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Sat, 5 Dec 2015 00:16:38 +0100 Subject: [PATCH 05/21] Remove useless test --- tests/unit/browser/test_adblock.py | 34 ++++++------------------------ 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index a3cb6048b..4204c31f4 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -97,38 +97,16 @@ class TestGetFileObj: class TestIsWhitelistedHost: """Test function adblock.is_whitelisted_host.""" - # FIXME Error since we deleted host-blocking-whitelist - # If we don't remove host-block-whitelist, test behaves as in a mismatch - # def test_with_no_whitelist(self): - # config_obj = config.ConfigManager(configdir=None, - # fname=None, - # relaxed=True) - # config_obj.remove_option('content','host-blocking-whitelist') - # objreg.register('config', config_obj) - # assert adblock.is_whitelisted_host('pimpmytest.com') == False - # objreg.delete('config') - - def test_with_match(self): + def test_with_match(self, config_stub): """Given host is in the whitelist.""" - config_obj = config.ConfigManager(configdir=None, fname=None, - relaxed=True) - config_obj.set_command(0, section_='content', - option='host-blocking-whitelist', - value='qutebrowser.org') - objreg.register('config', config_obj) + config_stub.data = {'content': {'host-blocking-whitelist': ['qutebrowser.org']}} assert adblock.is_whitelisted_host('qutebrowser.org') - objreg.delete('config') - def test_without_match(self): + def test_without_match(self, config_stub): """Given host is not in the whitelist.""" - config_obj = config.ConfigManager(configdir=None, fname=None, - relaxed=True) - config_obj.set_command(0, section_='content', - option='host-blocking-whitelist', - value='cutebrowser.org') - objreg.register('config', config_obj) - assert not adblock.is_whitelisted_host('qutebrowser.org') - objreg.delete('config') + config_stub.data = {'content': + {'host-blocking-whitelist':['qutebrowser.org']}} + assert not adblock.is_whitelisted_host('cutebrowser.org') class TestHostBlocker: From cc946ba6e629142bf22cb7334b4be19138da6923 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Sat, 5 Dec 2015 01:09:11 +0100 Subject: [PATCH 06/21] implementation of config_stub fixture --- tests/unit/browser/test_adblock.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 4204c31f4..02caca581 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -26,8 +26,7 @@ import zipfile import pytest from qutebrowser.browser import adblock -from qutebrowser.config import config -from qutebrowser.utils import objreg +from qutebrowser.config import configexc def create_text_files(files_names, directory): @@ -97,19 +96,25 @@ class TestGetFileObj: class TestIsWhitelistedHost: """Test function adblock.is_whitelisted_host.""" + def test_without_option(self, config_stub): + """Option host-blocking-whitelist does not exist""" + config_stub.data = {'content': {}} + with pytest.raises(configexc.NoOptionError): + adblock.is_whitelisted_host('qutebrowser.org') + def test_with_match(self, config_stub): """Given host is in the whitelist.""" - config_stub.data = {'content': {'host-blocking-whitelist': ['qutebrowser.org']}} + config_stub.data = {'content': + {'host-blocking-whitelist': ['qutebrowser.org']}} assert adblock.is_whitelisted_host('qutebrowser.org') def test_without_match(self, config_stub): """Given host is not in the whitelist.""" config_stub.data = {'content': - {'host-blocking-whitelist':['qutebrowser.org']}} + {'host-blocking-whitelist':['qutebrowser.org']}} assert not adblock.is_whitelisted_host('cutebrowser.org') class TestHostBlocker: """Test for class HostBlocker.""" - # TODO pass From 5e5531f9249898d8a1d116e2c5e4209a6b1912ad Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Sat, 5 Dec 2015 01:21:32 +0100 Subject: [PATCH 07/21] Revert "Remove old pull request commits" This reverts commit 5369fc30bcd8a31edb072e612f993b9b497739d5. messed with git - revert --- qutebrowser/config/config.py | 2 +- qutebrowser/config/sections.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 42f9395e2..57837084f 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -619,7 +619,7 @@ class ConfigManager(QObject): optname = self.optionxform(optname) existed = optname in sectdict if existed: - del sectdict[optname] + sectdict.delete(optname) # WORKAROUND for https://bitbucket.org/logilab/pylint/issues/659/ self.get.cache_clear() # pylint: disable=no-member return existed diff --git a/qutebrowser/config/sections.py b/qutebrowser/config/sections.py index 2a20b5b80..0253777cc 100644 --- a/qutebrowser/config/sections.py +++ b/qutebrowser/config/sections.py @@ -72,6 +72,10 @@ class Section: """Get value keys.""" return self.values.keys() + def delete(self, key): + """Delete item with given key""" + del self.values[key] + def setv(self, layer, key, value, interpolated): """Set the value on a layer. From d5fc7e03899adcefba7b051f8845e3e633515997 Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Sat, 5 Dec 2015 01:29:20 +0100 Subject: [PATCH 08/21] Merge upstream/master --- README.asciidoc | 1 + qutebrowser/config/sections.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.asciidoc b/README.asciidoc index b8aa08aad..436f03409 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -183,6 +183,7 @@ Contributors, sorted by the number of commits in descending order: * Mathias Fussenegger * Fritz V155 Reichwald * Franz Fellner +* Corentin Jule * zwarag * xd1le * Tim Harder diff --git a/qutebrowser/config/sections.py b/qutebrowser/config/sections.py index 0253777cc..bedde27aa 100644 --- a/qutebrowser/config/sections.py +++ b/qutebrowser/config/sections.py @@ -73,7 +73,7 @@ class Section: return self.values.keys() def delete(self, key): - """Delete item with given key""" + """Delete item with given key.""" del self.values[key] def setv(self, layer, key, value, interpolated): From d55e6d7d7ee50afd10a8f8c78e5c67b449f0fedf Mon Sep 17 00:00:00 2001 From: Corentin Jule Date: Sat, 5 Dec 2015 01:35:20 +0100 Subject: [PATCH 09/21] Comply with PEP257 --- tests/unit/browser/test_adblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 02caca581..afc400d1d 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -97,7 +97,7 @@ class TestIsWhitelistedHost: """Test function adblock.is_whitelisted_host.""" def test_without_option(self, config_stub): - """Option host-blocking-whitelist does not exist""" + """Option host-blocking-whitelist does not exist.""" config_stub.data = {'content': {}} with pytest.raises(configexc.NoOptionError): adblock.is_whitelisted_host('qutebrowser.org') From 9da15ae2f960e4611c179b3514d58fc94cd6f857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sat, 5 Dec 2015 12:41:52 +0100 Subject: [PATCH 10/21] Correct is_whitelisted_host test --- tests/unit/browser/test_adblock.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index afc400d1d..1dfe55fd9 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -96,11 +96,10 @@ class TestGetFileObj: class TestIsWhitelistedHost: """Test function adblock.is_whitelisted_host.""" - def test_without_option(self, config_stub): - """Option host-blocking-whitelist does not exist.""" - config_stub.data = {'content': {}} - with pytest.raises(configexc.NoOptionError): - adblock.is_whitelisted_host('qutebrowser.org') + def test_without_hosts(self, config_stub): + """No hosts are whitelisted.""" + config_stub.data = {'content': {'host-blocking-whitelist': None}} + assert not adblock.is_whitelisted_host('qutebrowser.org') def test_with_match(self, config_stub): """Given host is in the whitelist.""" From 472585edd550f43f32de7aaf2d0890b136268196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sun, 6 Dec 2015 00:10:56 +0100 Subject: [PATCH 11/21] Add tests for HostBlocker class --- tests/unit/browser/test_adblock.py | 109 +++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 6 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 1dfe55fd9..d38468cd6 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -22,12 +22,57 @@ import os import zipfile +import shutil import pytest +from PyQt5.QtCore import pyqtSignal, QUrl, QObject + from qutebrowser.browser import adblock from qutebrowser.config import configexc +from qutebrowser.utils import objreg +UNDESIRED_HOSTS = ['badsite.org','verybadsite.com','worstsiteever.net'] + +class FakeDownloadItem(QObject): + """Mock browser.downloads.DownloadItem.""" + + finished = pyqtSignal() + + def __init__(self, fileobj): + super().__init__() + self.fileobj = fileobj + self.successful = True + + +class FakeDownloadManager: + """Mock browser.downloads.DownloadManager.""" + + def get(self, url, fileobj, **kwargs): + """Returns a FakeDownloadItem instance with a fileobj + copied from given fake url file.""" + download_item = FakeDownloadItem(fileobj) + with open(url.path(), 'rb') as fake_url_file : + # Ensure cursors are at position 0 before copying + fake_url_file.seek(0) + download_item.fileobj.seek(0) + shutil.copyfileobj(fake_url_file, download_item.fileobj) + return download_item + +@pytest.yield_fixture +def download_stub(win_registry): + """Register a FakeDownloadManager.""" + stub = FakeDownloadManager() + objreg.register('download-manager', stub, + scope='window', window='last-focused') + yield stub + objreg.delete('download-manager', scope='window', window='last-focused') + +@pytest.fixture +def data_tmpdir(monkeypatch, tmpdir): + """Use tmpdir as datadir""" + monkeypatch.setattr('qutebrowser.utils.standarddir.data', + lambda: str(tmpdir)) def create_text_files(files_names, directory): """Returns a list of text files created @@ -37,11 +82,10 @@ def create_text_files(files_names, directory): for file_name in files_names: test_file = os.path.join(directory, file_name) with open(test_file, 'w', encoding='utf-8') as current_file: - current_file.write('inside ' + file_name) + current_file.write('www.' + file_name + '.com') created_files.append(test_file) return created_files - def create_zipfile(files_names, directory): """Returns a zipfile populated with created files and its name.""" directory = str(directory) @@ -85,12 +129,12 @@ class TestGetFileObj: names = ['test_a', 'test_b', 'hosts', 'test_c'] zf_name = create_zipfile(names, tmpdir)[1] zipobj = open(zf_name, 'rb') - assert adblock.get_fileobj(zipobj).read() == "inside hosts" + assert adblock.get_fileobj(zipobj).read() == "www.hosts.com" def test_with_text_file(self, tmpdir): """File provided is not a zipfile.""" test_file = open(create_text_files(['testfile'], tmpdir)[0], 'rb') - assert adblock.get_fileobj(test_file).read() == "inside testfile" + assert adblock.get_fileobj(test_file).read() == "www.testfile.com" class TestIsWhitelistedHost: @@ -115,5 +159,58 @@ class TestIsWhitelistedHost: class TestHostBlocker: - """Test for class HostBlocker.""" - pass + """Tests for class HostBlocker.""" + + def test_without_datadir(self, config_stub, monkeypatch): + """No directory for data configured, no hosts file present.""" + monkeypatch.setattr('qutebrowser.utils.standarddir.data', + lambda: None) + host_blocker = adblock.HostBlocker() + assert host_blocker._hosts_file == None + + def test_with_datadir(self, config_stub, data_tmpdir, tmpdir): + #TODO Remove since now useless as already tested by test_update + host_blocker = adblock.HostBlocker() + hosts_file_path = os.path.join(str(tmpdir), 'blocked-hosts') + assert host_blocker._hosts_file == hosts_file_path + + def test_update_with_fake_url(self, config_stub, download_stub, + data_tmpdir, tmpdir, win_registry): + """Test update, checked Url host is in the new blocklist added by update + Remote Url is faked by a local file.""" + # Create blocklist and add it to config + blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) + with open(blocklist.path(), 'w', encoding='UTF-8') as hosts: + for path in UNDESIRED_HOSTS: + hosts.write(path + '\n') + config_stub.data = {'content': + {'host-block-lists': [blocklist], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + #Simulate download is finished + host_blocker._in_progress[0].finished.emit() + url_to_check = QUrl("www.ugly.verybadsite.com") + url_to_check.setHost("verybadsite.com") + assert host_blocker.is_blocked(url_to_check) + + def test_update_with_local_file(self, config_stub, download_stub, + data_tmpdir, tmpdir, win_registry): + """Test update, checked Url host is in the new blocklist added by update + Url is a local file.""" + # Create blocklist and add it to config + local_blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) + local_blocklist.setScheme("file") + with open(local_blocklist.path(), 'w', encoding='UTF-8') as hosts: + for path in UNDESIRED_HOSTS: + hosts.write(path + '\n') + config_stub.data = {'content': + {'host-block-lists': [local_blocklist], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + url_to_check = QUrl("www.ugly.verybadsite.com") + url_to_check.setHost("verybadsite.com") + assert host_blocker.is_blocked(url_to_check) From 1ba634969ae102dd59507c4662038a376ff021e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sun, 6 Dec 2015 00:42:33 +0100 Subject: [PATCH 12/21] Correct QUrl creation --- tests/unit/browser/test_adblock.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index d38468cd6..74b736662 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -174,8 +174,8 @@ class TestHostBlocker: hosts_file_path = os.path.join(str(tmpdir), 'blocked-hosts') assert host_blocker._hosts_file == hosts_file_path - def test_update_with_fake_url(self, config_stub, download_stub, - data_tmpdir, tmpdir, win_registry): + def test_update_with_url(self, config_stub, download_stub, + data_tmpdir, tmpdir, win_registry): """Test update, checked Url host is in the new blocklist added by update Remote Url is faked by a local file.""" # Create blocklist and add it to config @@ -191,16 +191,16 @@ class TestHostBlocker: host_blocker.adblock_update(0) #Simulate download is finished host_blocker._in_progress[0].finished.emit() - url_to_check = QUrl("www.ugly.verybadsite.com") - url_to_check.setHost("verybadsite.com") + url_to_check = QUrl("http://verybadsite.com") assert host_blocker.is_blocked(url_to_check) def test_update_with_local_file(self, config_stub, download_stub, data_tmpdir, tmpdir, win_registry): """Test update, checked Url host is in the new blocklist added by update Url is a local file.""" - # Create blocklist and add it to config + # Create blocklist local_blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) + # Declare the blacklist as a local file local_blocklist.setScheme("file") with open(local_blocklist.path(), 'w', encoding='UTF-8') as hosts: for path in UNDESIRED_HOSTS: @@ -211,6 +211,5 @@ class TestHostBlocker: 'host-blocking-whitelist': None}} host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) - url_to_check = QUrl("www.ugly.verybadsite.com") - url_to_check.setHost("verybadsite.com") + url_to_check = QUrl("http://verybadsite.com") assert host_blocker.is_blocked(url_to_check) From ac3d0b9a4c1124a1915441c69fad02e88706556c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sun, 6 Dec 2015 00:51:44 +0100 Subject: [PATCH 13/21] Comply with pylint --- tests/unit/browser/test_adblock.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 74b736662..0632e3a34 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -29,10 +29,11 @@ import pytest from PyQt5.QtCore import pyqtSignal, QUrl, QObject from qutebrowser.browser import adblock -from qutebrowser.config import configexc from qutebrowser.utils import objreg -UNDESIRED_HOSTS = ['badsite.org','verybadsite.com','worstsiteever.net'] + +UNDESIRED_HOSTS = ['badsite.org', 'verybadsite.com', 'worstsiteever.net'] + class FakeDownloadItem(QObject): """Mock browser.downloads.DownloadItem.""" @@ -52,7 +53,7 @@ class FakeDownloadManager: """Returns a FakeDownloadItem instance with a fileobj copied from given fake url file.""" download_item = FakeDownloadItem(fileobj) - with open(url.path(), 'rb') as fake_url_file : + with open(url.path(), 'rb') as fake_url_file: # Ensure cursors are at position 0 before copying fake_url_file.seek(0) download_item.fileobj.seek(0) @@ -166,7 +167,7 @@ class TestHostBlocker: monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: None) host_blocker = adblock.HostBlocker() - assert host_blocker._hosts_file == None + assert host_blocker._hosts_file is None def test_with_datadir(self, config_stub, data_tmpdir, tmpdir): #TODO Remove since now useless as already tested by test_update @@ -176,7 +177,7 @@ class TestHostBlocker: def test_update_with_url(self, config_stub, download_stub, data_tmpdir, tmpdir, win_registry): - """Test update, checked Url host is in the new blocklist added by update + """Test update, checked Url is in the new blocklist added by update Remote Url is faked by a local file.""" # Create blocklist and add it to config blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) @@ -196,7 +197,7 @@ class TestHostBlocker: def test_update_with_local_file(self, config_stub, download_stub, data_tmpdir, tmpdir, win_registry): - """Test update, checked Url host is in the new blocklist added by update + """Test update, checked Url is in the new blocklist added by update Url is a local file.""" # Create blocklist local_blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) From 8bff518ba4bcb2799eeea49202a122ffcc94969d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sun, 6 Dec 2015 15:35:10 +0100 Subject: [PATCH 14/21] Refactor and add tests --- tests/unit/browser/test_adblock.py | 248 ++++++++++++++++------------- 1 file changed, 135 insertions(+), 113 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 0632e3a34..9f5866f46 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -30,9 +30,20 @@ from PyQt5.QtCore import pyqtSignal, QUrl, QObject from qutebrowser.browser import adblock from qutebrowser.utils import objreg +from qutebrowser.commands import cmdexc -UNDESIRED_HOSTS = ['badsite.org', 'verybadsite.com', 'worstsiteever.net'] +WHITELISTED_HOSTS = ['qutebrowser.org', 'badsite.org'] +BLOCKED_HOSTS = ['badsite.org', 'localhost', + 'verybadsite.com', 'worstsiteever.net'] +URLS_TO_CHECK = ['http://verybadsite.com', 'http://badsite.org', + 'http://localhost', 'http://qutebrowser.org', + 'http://a.com', 'http://b.com'] + +class BaseDirStub: + """Mock for objreg.get['args'] called in adblock.HostBlocker.read_hosts.""" + def __init__(self): + self.basedir = None class FakeDownloadItem(QObject): @@ -60,6 +71,7 @@ class FakeDownloadManager: shutil.copyfileobj(fake_url_file, download_item.fileobj) return download_item + @pytest.yield_fixture def download_stub(win_registry): """Register a FakeDownloadManager.""" @@ -69,148 +81,158 @@ def download_stub(win_registry): yield stub objreg.delete('download-manager', scope='window', window='last-focused') +@pytest.yield_fixture +def basedir(): + """Register a Fake basedir.""" + args = BaseDirStub() + objreg.register('args', args) + yield args + objreg.delete('args') + @pytest.fixture def data_tmpdir(monkeypatch, tmpdir): """Use tmpdir as datadir""" monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: str(tmpdir)) -def create_text_files(files_names, directory): - """Returns a list of text files created - with given names in given directory.""" +def create_zipfile(files, directory): + """Returns a zipfile populated with given files and its name.""" directory = str(directory) - created_files = [] - for file_name in files_names: - test_file = os.path.join(directory, file_name) - with open(test_file, 'w', encoding='utf-8') as current_file: - current_file.write('www.' + file_name + '.com') - created_files.append(test_file) - return created_files - -def create_zipfile(files_names, directory): - """Returns a zipfile populated with created files and its name.""" - directory = str(directory) - files = create_text_files(files_names, directory) - # include created files in a ZipFile - zipfile_name = os.path.join(directory, 'test.zip') - with zipfile.ZipFile(zipfile_name, 'w') as new_zipfile: + zipfile_path = os.path.join(directory, 'test.zip') + with zipfile.ZipFile(zipfile_path, 'w') as new_zipfile: for file_name in files: new_zipfile.write(file_name, arcname=os.path.basename(file_name)) # Removes path from file name - return new_zipfile, zipfile_name + return new_zipfile, zipfile_path +def create_blocklist(blocked_hosts, name, directory): + """Returns a QUrl instance linking to a file with given name in given + directory which contains a list of given blocked_hosts.""" + blocklist = QUrl(os.path.join(str(directory), name)) + with open(blocklist.path(), 'w', encoding='UTF-8') as hosts: + for path in blocked_hosts: + hosts.write(path + '\n') + return blocklist -class TestGuessZipFilename: - """Test function adblock.guess_zip_filename().""" - - def test_with_single_file(self, tmpdir): - """Zip provided only contains a single file.""" - zf = create_zipfile(['test_a'], tmpdir)[0] - assert adblock.guess_zip_filename(zf) == 'test_a' - - def test_with_multiple_files(self, tmpdir): - """Zip provided contains multiple files including hosts.""" - names = ['test_a', 'test_b', 'hosts', 'test_c'] - zf = create_zipfile(names, tmpdir)[0] - assert adblock.guess_zip_filename(zf) == 'hosts' - - def test_without_hosts_file(self, tmpdir): - """Zip provided does not contain any hosts file.""" - names = ['test_a', 'test_b', 'test_d', 'test_c'] - zf = create_zipfile(names, tmpdir)[0] - with pytest.raises(FileNotFoundError): - adblock.guess_zip_filename(zf) - - -class TestGetFileObj: - """Test Function adblock.get_fileobj().""" - - def test_with_zipfile(self, tmpdir): - """File provided is a zipfile.""" - names = ['test_a', 'test_b', 'hosts', 'test_c'] - zf_name = create_zipfile(names, tmpdir)[1] - zipobj = open(zf_name, 'rb') - assert adblock.get_fileobj(zipobj).read() == "www.hosts.com" - - def test_with_text_file(self, tmpdir): - """File provided is not a zipfile.""" - test_file = open(create_text_files(['testfile'], tmpdir)[0], 'rb') - assert adblock.get_fileobj(test_file).read() == "www.testfile.com" - - -class TestIsWhitelistedHost: - """Test function adblock.is_whitelisted_host.""" - - def test_without_hosts(self, config_stub): - """No hosts are whitelisted.""" - config_stub.data = {'content': {'host-blocking-whitelist': None}} - assert not adblock.is_whitelisted_host('qutebrowser.org') - - def test_with_match(self, config_stub): - """Given host is in the whitelist.""" - config_stub.data = {'content': - {'host-blocking-whitelist': ['qutebrowser.org']}} - assert adblock.is_whitelisted_host('qutebrowser.org') - - def test_without_match(self, config_stub): - """Given host is not in the whitelist.""" - config_stub.data = {'content': - {'host-blocking-whitelist':['qutebrowser.org']}} - assert not adblock.is_whitelisted_host('cutebrowser.org') +def assert_urls(host_blocker, blocked_hosts, whitelisted_hosts, urls_to_check): + """Test if urls_to_check are effectively blocked or not by HostBlocker.""" + for str_url in urls_to_check: + url = QUrl(str_url) + host = url.host() + if host in blocked_hosts and host not in whitelisted_hosts: + assert host_blocker.is_blocked(url) + else: + assert not host_blocker.is_blocked(url) class TestHostBlocker: """Tests for class HostBlocker.""" - def test_without_datadir(self, config_stub, monkeypatch): - """No directory for data configured, no hosts file present.""" + def test_unsuccessful_update(self, config_stub, monkeypatch, win_registry): + """No directory for data configured so no hosts file exists.""" monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: None) host_blocker = adblock.HostBlocker() - assert host_blocker._hosts_file is None + with pytest.raises(cmdexc.CommandError): + host_blocker.adblock_update(0) + assert host_blocker.read_hosts() is None - def test_with_datadir(self, config_stub, data_tmpdir, tmpdir): - #TODO Remove since now useless as already tested by test_update + def test_host_blocking_disabled(self, basedir, config_stub, download_stub, + data_tmpdir, tmpdir, win_registry): + """Assert that no host is blocked when blocking is disabled.""" + blocklist = create_blocklist(BLOCKED_HOSTS, 'hosts.txt', tmpdir) + config_stub.data = {'content': + {'host-block-lists': [blocklist], + 'host-blocking-enabled': False}} host_blocker = adblock.HostBlocker() - hosts_file_path = os.path.join(str(tmpdir), 'blocked-hosts') - assert host_blocker._hosts_file == hosts_file_path + host_blocker.adblock_update(0) + host_blocker.read_hosts() + for strurl in URLS_TO_CHECK: + url = QUrl(strurl) + assert not host_blocker.is_blocked(url) - def test_update_with_url(self, config_stub, download_stub, - data_tmpdir, tmpdir, win_registry): - """Test update, checked Url is in the new blocklist added by update - Remote Url is faked by a local file.""" - # Create blocklist and add it to config - blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) - with open(blocklist.path(), 'w', encoding='UTF-8') as hosts: - for path in UNDESIRED_HOSTS: - hosts.write(path + '\n') + def test_update_no_blocklist(self, config_stub, download_stub, + data_tmpdir, basedir, win_registry): + """Assert that no host is blocked when no blocklist exists.""" + config_stub.data = {'content': + {'host-block-lists' : None, + 'host-blocking-enabled': True}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + host_blocker.read_hosts() + for strurl in URLS_TO_CHECK: + url = QUrl(strurl) + assert not host_blocker.is_blocked(url) + + def test_successful_update(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """ + Test successfull update with : + - fake remote text file + - local text file + - fake remote zip file (contains 1 text file) + - fake remote zip file (contains 2 text files, 0 named hosts) + - fake remote zip file (contains 2 text files, 1 named hosts). + """ + + # Primary test with fake remote text file + blocklist = create_blocklist(BLOCKED_HOSTS, 'blocklist.txt', tmpdir) config_stub.data = {'content': {'host-block-lists': [blocklist], 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} + 'host-blocking-whitelist': WHITELISTED_HOSTS}} host_blocker = adblock.HostBlocker() + whitelisted = list(host_blocker.WHITELISTED) + WHITELISTED_HOSTS + # host file has not been created yet, message run-adblock will be sent + # host_blocker.read_hosts() host_blocker.adblock_update(0) #Simulate download is finished host_blocker._in_progress[0].finished.emit() - url_to_check = QUrl("http://verybadsite.com") - assert host_blocker.is_blocked(url_to_check) + host_blocker.read_hosts() + assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) - def test_update_with_local_file(self, config_stub, download_stub, - data_tmpdir, tmpdir, win_registry): - """Test update, checked Url is in the new blocklist added by update - Url is a local file.""" - # Create blocklist - local_blocklist = QUrl(os.path.join(str(tmpdir), 'new_hosts.txt')) - # Declare the blacklist as a local file + # Alternative test with local file + local_blocklist = blocklist local_blocklist.setScheme("file") - with open(local_blocklist.path(), 'w', encoding='UTF-8') as hosts: - for path in UNDESIRED_HOSTS: - hosts.write(path + '\n') - config_stub.data = {'content': - {'host-block-lists': [local_blocklist], - 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} - host_blocker = adblock.HostBlocker() + config_stub.set('content', 'host-block-lists', [local_blocklist]) host_blocker.adblock_update(0) - url_to_check = QUrl("http://verybadsite.com") - assert host_blocker.is_blocked(url_to_check) + host_blocker.read_hosts() + assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) + + # Alternative test with fake remote zip file containing one file + zip_blocklist_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) + config_stub.set('content', 'host-block-lists', [zip_blocklist_url]) + host_blocker.adblock_update(0) + #Simulate download is finished + host_blocker._in_progress[0].finished.emit() + host_blocker.read_hosts() + assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) + + # Alternative test with fake remote zip file containing multiple files + # FIXME adblock.guess_zip_filename should raise FileNotFound Error + # as no files in the zip are called hosts + first_file = create_blocklist(BLOCKED_HOSTS, 'file1.txt', tmpdir) + second_file = create_blocklist(['a.com', 'b.com'], 'file2.txt', tmpdir) + files_to_zip = [first_file.path(), second_file.path()] + zip_blocklist_path = create_zipfile(files_to_zip, tmpdir)[1] + zip_blocklist_url = QUrl(zip_blocklist_path) + config_stub.set('content', 'host-block-lists', [zip_blocklist_url]) + host_blocker.adblock_update(0) + #Simulate download is finished + with pytest.raises(FileNotFoundError): + host_blocker._in_progress[0].finished.emit() + host_blocker.read_hosts() + + # Alternative test with fake remote zip file containing multiple files + # Including a file called hosts + first_file = create_blocklist(BLOCKED_HOSTS, 'hosts.txt', tmpdir) + second_file = create_blocklist(['a.com', 'b.com'], 'file2.txt', tmpdir) + files_to_zip = [first_file.path(), second_file.path()] + zip_blocklist_path = create_zipfile(files_to_zip, tmpdir)[1] + zip_blocklist_url = QUrl(zip_blocklist_path) + config_stub.set('content', 'host-block-lists', [zip_blocklist_url]) + host_blocker.adblock_update(0) + #Simulate download is finished + host_blocker._in_progress[0].finished.emit() + host_blocker.read_hosts() + assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) From a24a7790cd64b9715908cb57741642390bf548f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sun, 6 Dec 2015 19:33:35 +0100 Subject: [PATCH 15/21] Improve code readability --- tests/unit/browser/test_adblock.py | 271 ++++++++++++++++++----------- 1 file changed, 172 insertions(+), 99 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 9f5866f46..2f5fc1134 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -33,18 +33,47 @@ from qutebrowser.utils import objreg from qutebrowser.commands import cmdexc -WHITELISTED_HOSTS = ['qutebrowser.org', 'badsite.org'] -BLOCKED_HOSTS = ['badsite.org', 'localhost', - 'verybadsite.com', 'worstsiteever.net'] -URLS_TO_CHECK = ['http://verybadsite.com', 'http://badsite.org', - 'http://localhost', 'http://qutebrowser.org', - 'http://a.com', 'http://b.com'] +WHITELISTED_HOSTS = ['qutebrowser.org', + 'mediumsite.org'] +BLOCKED_HOSTS = ['localhost', + 'mediumsite.io', + 'badsite.org', + 'verybadsite.com', + 'worstsiteever.net'] + +URLS_TO_CHECK = ['http://verybadsite.com', + 'http://badsite.org', + 'http://localhost', + 'http://qutebrowser.org', + 'http://testa.com', + 'http://testb.com'] + + +@pytest.fixture +def data_tmpdir(monkeypatch, tmpdir): + """Set tmpdir as datadir""" + monkeypatch.setattr('qutebrowser.utils.standarddir.data', + lambda: str(tmpdir)) + + +# XXX Why does read_hosts needs basedir to be None +# in order to print message 'run adblock-update to read host blocklist' ? +# browser/adblock.py line 138 class BaseDirStub: - """Mock for objreg.get['args'] called in adblock.HostBlocker.read_hosts.""" + """Mock for objreg.get['args'] + called in adblock.HostBlocker.read_hosts.""" def __init__(self): self.basedir = None +@pytest.yield_fixture +def basedir(): + """Register a Fake basedir.""" + args = BaseDirStub() + objreg.register('args', args) + yield args + objreg.delete('args') + class FakeDownloadItem(QObject): """Mock browser.downloads.DownloadItem.""" @@ -56,7 +85,6 @@ class FakeDownloadItem(QObject): self.fileobj = fileobj self.successful = True - class FakeDownloadManager: """Mock browser.downloads.DownloadManager.""" @@ -71,7 +99,6 @@ class FakeDownloadManager: shutil.copyfileobj(fake_url_file, download_item.fileobj) return download_item - @pytest.yield_fixture def download_stub(win_registry): """Register a FakeDownloadManager.""" @@ -81,19 +108,7 @@ def download_stub(win_registry): yield stub objreg.delete('download-manager', scope='window', window='last-focused') -@pytest.yield_fixture -def basedir(): - """Register a Fake basedir.""" - args = BaseDirStub() - objreg.register('args', args) - yield args - objreg.delete('args') -@pytest.fixture -def data_tmpdir(monkeypatch, tmpdir): - """Use tmpdir as datadir""" - monkeypatch.setattr('qutebrowser.utils.standarddir.data', - lambda: str(tmpdir)) def create_zipfile(files, directory): """Returns a zipfile populated with given files and its name.""" @@ -105,7 +120,7 @@ def create_zipfile(files, directory): # Removes path from file name return new_zipfile, zipfile_path -def create_blocklist(blocked_hosts, name, directory): +def create_blocklist(directory, blocked_hosts=BLOCKED_HOSTS, name='hosts'): """Returns a QUrl instance linking to a file with given name in given directory which contains a list of given blocked_hosts.""" blocklist = QUrl(os.path.join(str(directory), name)) @@ -114,8 +129,14 @@ def create_blocklist(blocked_hosts, name, directory): hosts.write(path + '\n') return blocklist -def assert_urls(host_blocker, blocked_hosts, whitelisted_hosts, urls_to_check): - """Test if urls_to_check are effectively blocked or not by HostBlocker.""" +def assert_urls(host_blocker, + blocked_hosts=BLOCKED_HOSTS, + whitelisted_hosts=WHITELISTED_HOSTS, + urls_to_check=URLS_TO_CHECK): + """Test if urls_to_check are effectively blocked or not by HostBlocker + Url in blocked_hosts and not in whitelisted_hosts should be blocked + All other Urls should not be blocked.""" + whitelisted_hosts += list(host_blocker.WHITELISTED) for str_url in urls_to_check: url = QUrl(str_url) host = url.host() @@ -125,114 +146,166 @@ def assert_urls(host_blocker, blocked_hosts, whitelisted_hosts, urls_to_check): assert not host_blocker.is_blocked(url) -class TestHostBlocker: - """Tests for class HostBlocker.""" +class TestHostBlockerUpdate: - def test_unsuccessful_update(self, config_stub, monkeypatch, win_registry): - """No directory for data configured so no hosts file exists.""" + """Tests for function adblock_update of class HostBlocker.""" + + def test_without_datadir(self, config_stub, tmpdir, + monkeypatch, win_registry): + """No directory for data configured so no hosts file exists. + CommandError is raised by adblock_update + Ensure no url is blocked.""" + blocklist = create_blocklist(tmpdir) + config_stub.data = {'content': + {'host-block-lists': [blocklist], + 'host-blocking-enabled': True}} monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: None) host_blocker = adblock.HostBlocker() with pytest.raises(cmdexc.CommandError): host_blocker.adblock_update(0) - assert host_blocker.read_hosts() is None + host_blocker.read_hosts() + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) - def test_host_blocking_disabled(self, basedir, config_stub, download_stub, - data_tmpdir, tmpdir, win_registry): - """Assert that no host is blocked when blocking is disabled.""" - blocklist = create_blocklist(BLOCKED_HOSTS, 'hosts.txt', tmpdir) + def test_disabled_blocking(self, basedir, config_stub, download_stub, + data_tmpdir, tmpdir, win_registry): + """Ensure that no url is blocked when host blocking is disabled.""" + blocklist = create_blocklist(tmpdir) config_stub.data = {'content': {'host-block-lists': [blocklist], 'host-blocking-enabled': False}} host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) + host_blocker._in_progress[0].finished.emit() host_blocker.read_hosts() - for strurl in URLS_TO_CHECK: - url = QUrl(strurl) - assert not host_blocker.is_blocked(url) + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) - def test_update_no_blocklist(self, config_stub, download_stub, - data_tmpdir, basedir, win_registry): - """Assert that no host is blocked when no blocklist exists.""" + def test_no_blocklist(self, config_stub, download_stub, + data_tmpdir, basedir, tmpdir, win_registry): + """Ensure no host is blocked when no blocklist exists.""" config_stub.data = {'content': {'host-block-lists' : None, 'host-blocking-enabled': True}} host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) host_blocker.read_hosts() - for strurl in URLS_TO_CHECK: - url = QUrl(strurl) - assert not host_blocker.is_blocked(url) + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) - def test_successful_update(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """ - Test successfull update with : - - fake remote text file - - local text file - - fake remote zip file (contains 1 text file) - - fake remote zip file (contains 2 text files, 0 named hosts) - - fake remote zip file (contains 2 text files, 1 named hosts). - """ - - # Primary test with fake remote text file - blocklist = create_blocklist(BLOCKED_HOSTS, 'blocklist.txt', tmpdir) + def test_remote_text(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """Update with single fakely remote text blocklist. + Ensure urls from hosts in this blocklist get blocked.""" + blocklist = create_blocklist(tmpdir) config_stub.data = {'content': {'host-block-lists': [blocklist], 'host-blocking-enabled': True, - 'host-blocking-whitelist': WHITELISTED_HOSTS}} + 'host-blocking-whitelist': None}} host_blocker = adblock.HostBlocker() - whitelisted = list(host_blocker.WHITELISTED) + WHITELISTED_HOSTS - # host file has not been created yet, message run-adblock will be sent - # host_blocker.read_hosts() host_blocker.adblock_update(0) - #Simulate download is finished + # Simulate download is finished + # XXX Is it ok to use private attribute hostblocker._in_progress ? host_blocker._in_progress[0].finished.emit() host_blocker.read_hosts() - assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) + assert_urls(host_blocker, whitelisted_hosts=[]) - # Alternative test with local file - local_blocklist = blocklist - local_blocklist.setScheme("file") - config_stub.set('content', 'host-block-lists', [local_blocklist]) + def test_remote_zip_single(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """Update with single fakely remote zip containing one blocklist file. + Ensure urls from hosts in this blocklist get blocked.""" + blocklist = create_blocklist(tmpdir) + zip_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) + config_stub.data = {'content': + {'host-block-lists': [zip_url], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) - host_blocker.read_hosts() - assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) - - # Alternative test with fake remote zip file containing one file - zip_blocklist_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) - config_stub.set('content', 'host-block-lists', [zip_blocklist_url]) - host_blocker.adblock_update(0) - #Simulate download is finished host_blocker._in_progress[0].finished.emit() host_blocker.read_hosts() - assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) + assert_urls(host_blocker, whitelisted_hosts=[]) - # Alternative test with fake remote zip file containing multiple files - # FIXME adblock.guess_zip_filename should raise FileNotFound Error - # as no files in the zip are called hosts - first_file = create_blocklist(BLOCKED_HOSTS, 'file1.txt', tmpdir) - second_file = create_blocklist(['a.com', 'b.com'], 'file2.txt', tmpdir) - files_to_zip = [first_file.path(), second_file.path()] - zip_blocklist_path = create_zipfile(files_to_zip, tmpdir)[1] - zip_blocklist_url = QUrl(zip_blocklist_path) - config_stub.set('content', 'host-block-lists', [zip_blocklist_url]) + # FIXME adblock.guess_zip_filename should raise FileNotFound Error + # as no files in the zip are called hosts + def test_remote_zip_multi1(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """Update with single fakely remote zip containing two files. + None of them is called hosts, FileNotFoundError should be raised.""" + file1 = create_blocklist(tmpdir, name='file1.txt') + file2_hosts = ['testa.com', 'testb.com'] + file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') + files_to_zip = [file1.path(), file2.path()] + zip_path = create_zipfile(files_to_zip, tmpdir)[1] + zip_url = QUrl(zip_path) + config_stub.data = {'content': + {'host-block-lists': [zip_url], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) - #Simulate download is finished - with pytest.raises(FileNotFoundError): - host_blocker._in_progress[0].finished.emit() - host_blocker.read_hosts() - - # Alternative test with fake remote zip file containing multiple files - # Including a file called hosts - first_file = create_blocklist(BLOCKED_HOSTS, 'hosts.txt', tmpdir) - second_file = create_blocklist(['a.com', 'b.com'], 'file2.txt', tmpdir) - files_to_zip = [first_file.path(), second_file.path()] - zip_blocklist_path = create_zipfile(files_to_zip, tmpdir)[1] - zip_blocklist_url = QUrl(zip_blocklist_path) - config_stub.set('content', 'host-block-lists', [zip_blocklist_url]) - host_blocker.adblock_update(0) - #Simulate download is finished host_blocker._in_progress[0].finished.emit() host_blocker.read_hosts() - assert_urls(host_blocker, BLOCKED_HOSTS, whitelisted, URLS_TO_CHECK) + #with pytest.raises(FileNotFoundError): + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) + + def test_remote_zip_multi2(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """Update with single fakely remote zip containing two files. + One of them is called hosts and should be used as blocklist. + Ensure urls from hosts in this blocklist get blocked + and the hosts from the other file are not.""" + file1 = create_blocklist(tmpdir, name='hosts.txt') + file2_hosts = ['testa.com', 'testb.com'] + file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') + files_to_zip = [file1.path(), file2.path()] + zip_path = create_zipfile(files_to_zip, tmpdir)[1] + zip_url = QUrl(zip_path) + config_stub.data = {'content': + {'host-block-lists': [zip_url], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + host_blocker._in_progress[0].finished.emit() + host_blocker.read_hosts() + assert_urls(host_blocker, whitelisted_hosts=[]) + + def test_local_text(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """Update with single local text blocklist. + Ensure urls from hosts in this blocklist get blocked.""" + blocklist = create_blocklist(tmpdir) + blocklist.setScheme("file") + config_stub.data = {'content': + {'host-block-lists': [blocklist], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + host_blocker.read_hosts() + assert_urls(host_blocker, whitelisted_hosts=[]) + + def test_local_zip_multi(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """Update with single local zip containing two files. + One of them is called hosts and should be used as blocklist. + Ensure urls from hosts in this blocklist get blocked + and the hosts from the other file are not.""" + file1 = create_blocklist(tmpdir, name='hosts.txt') + file2_hosts = ['testa.com', 'testb.com'] + file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') + files_to_zip = [file1.path(), file2.path()] + zip_path = create_zipfile(files_to_zip, tmpdir)[1] + zip_url = QUrl(zip_path) + zip_url.setScheme('file') + config_stub.data = {'content': + {'host-block-lists': [zip_url], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + host_blocker.read_hosts() + assert_urls(host_blocker, whitelisted_hosts=[]) From 6be0ff67f76eb389d16d6fa54e105419c33c9575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Mon, 7 Dec 2015 00:09:51 +0100 Subject: [PATCH 16/21] Replace some tests with a generic host blocking list --- tests/unit/browser/test_adblock.py | 327 ++++++++++++++++++----------- 1 file changed, 204 insertions(+), 123 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 2f5fc1134..78019d301 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -32,23 +32,25 @@ from qutebrowser.browser import adblock from qutebrowser.utils import objreg from qutebrowser.commands import cmdexc +WHITELISTED_HOSTS = ('qutebrowser.org', 'goodhost.gov') -WHITELISTED_HOSTS = ['qutebrowser.org', - 'mediumsite.org'] - -BLOCKED_HOSTS = ['localhost', - 'mediumsite.io', - 'badsite.org', - 'verybadsite.com', - 'worstsiteever.net'] - -URLS_TO_CHECK = ['http://verybadsite.com', - 'http://badsite.org', - 'http://localhost', - 'http://qutebrowser.org', - 'http://testa.com', - 'http://testb.com'] +BLOCKED_HOSTS = ('verygoodhost.com', + 'goodhost.gov', + 'mediumhost.io', + 'malware.badhost.org', + '4-verybadhost.com', + 'ads.worsthostever.net', + 'localhost') +URLS_TO_CHECK = ('http://localhost', + 'ftp://goodhost.gov', + 'http://mediumhost.io', + 'http://malware.badhost.org', + 'http://4-verybadhost.com', + 'http://ads.worsthostever.net', + 'http://verygoodhost.com', + 'ftp://perfecthost.com', + 'http://qutebrowser.org') @pytest.fixture def data_tmpdir(monkeypatch, tmpdir): @@ -110,23 +112,31 @@ def download_stub(win_registry): -def create_zipfile(files, directory): +def create_zipfile(files, directory, zipname='test'): """Returns a zipfile populated with given files and its name.""" directory = str(directory) - zipfile_path = os.path.join(directory, 'test.zip') + zipfile_path = os.path.join(directory, zipname + '.zip') with zipfile.ZipFile(zipfile_path, 'w') as new_zipfile: for file_name in files: new_zipfile.write(file_name, arcname=os.path.basename(file_name)) # Removes path from file name return new_zipfile, zipfile_path -def create_blocklist(directory, blocked_hosts=BLOCKED_HOSTS, name='hosts'): +def create_blocklist(directory, blocked_hosts=BLOCKED_HOSTS, + name='hosts', line_format=None): """Returns a QUrl instance linking to a file with given name in given directory which contains a list of given blocked_hosts.""" blocklist = QUrl(os.path.join(str(directory), name)) with open(blocklist.path(), 'w', encoding='UTF-8') as hosts: - for path in blocked_hosts: - hosts.write(path + '\n') + hosts.write('# Blocked Hosts List #\n\n') + if line_format == 'etc_hosts': + for path in blocked_hosts: + hosts.write('127.0.0.1 ' + path + '\n') + elif line_format == 'no_hosts': + hosts.write('This file is not a hosts file') + else: #one host per line format + for path in blocked_hosts: + hosts.write(path + '\n') return blocklist def assert_urls(host_blocker, @@ -136,7 +146,7 @@ def assert_urls(host_blocker, """Test if urls_to_check are effectively blocked or not by HostBlocker Url in blocked_hosts and not in whitelisted_hosts should be blocked All other Urls should not be blocked.""" - whitelisted_hosts += list(host_blocker.WHITELISTED) + whitelisted_hosts = list(whitelisted_hosts) + list(host_blocker.WHITELISTED) for str_url in urls_to_check: url = QUrl(str_url) host = url.host() @@ -150,14 +160,34 @@ class TestHostBlockerUpdate: """Tests for function adblock_update of class HostBlocker.""" + def generic_blocklists(self, directory): + file1 = create_blocklist(directory, BLOCKED_HOSTS[5:], 'hosts', 'etc_hosts') + file2 = create_blocklist(directory, name='README', line_format='no_hosts') + file3 = create_blocklist(directory, BLOCKED_HOSTS[:2], 'false_positive') + files_to_zip = [file1.path(), file2.path(), file3.path()] + zip_path = create_zipfile(files_to_zip, directory, 'block1')[1] + remote_blocklist1 = QUrl(zip_path) + + blocklist2 = create_blocklist(directory, [BLOCKED_HOSTS[4]], 'malwarelist', 'etc_hosts') + zip_path = create_zipfile([blocklist2.path()], directory, 'block2')[1] + remote_blocklist2 = QUrl(zip_path) + + # A local list with one host per line + local_blocklist3 = create_blocklist(directory, [BLOCKED_HOSTS[3]], 'mycustomblocklist', 'etc_hosts') + local_blocklist3.setScheme('file') + + # A list that cannot be read due to its formatting + remote_blocklist4 = create_blocklist(directory, [BLOCKED_HOSTS[2]], 'badlist', 'no_hosts') + + return [remote_blocklist1, remote_blocklist2, local_blocklist3, remote_blocklist4] + def test_without_datadir(self, config_stub, tmpdir, monkeypatch, win_registry): """No directory for data configured so no hosts file exists. CommandError is raised by adblock_update Ensure no url is blocked.""" - blocklist = create_blocklist(tmpdir) config_stub.data = {'content': - {'host-block-lists': [blocklist], + {'host-block-lists': self.generic_blocklists(tmpdir), 'host-blocking-enabled': True}} monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: None) @@ -171,9 +201,8 @@ class TestHostBlockerUpdate: def test_disabled_blocking(self, basedir, config_stub, download_stub, data_tmpdir, tmpdir, win_registry): """Ensure that no url is blocked when host blocking is disabled.""" - blocklist = create_blocklist(tmpdir) config_stub.data = {'content': - {'host-block-lists': [blocklist], + {'host-block-lists': self.generic_blocklists(tmpdir), 'host-blocking-enabled': False}} host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) @@ -194,118 +223,170 @@ class TestHostBlockerUpdate: for str_url in URLS_TO_CHECK: assert not host_blocker.is_blocked(QUrl(str_url)) - def test_remote_text(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """Update with single fakely remote text blocklist. - Ensure urls from hosts in this blocklist get blocked.""" - blocklist = create_blocklist(tmpdir) + def test_successful_update(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): config_stub.data = {'content': - {'host-block-lists': [blocklist], + {'host-block-lists': self.generic_blocklists(tmpdir), 'host-blocking-enabled': True, 'host-blocking-whitelist': None}} host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) # Simulate download is finished # XXX Is it ok to use private attribute hostblocker._in_progress ? - host_blocker._in_progress[0].finished.emit() + while host_blocker._in_progress != []: + host_blocker._in_progress[0].finished.emit() host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) + assert_urls(host_blocker, BLOCKED_HOSTS[3:], whitelisted_hosts=[]) - def test_remote_zip_single(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """Update with single fakely remote zip containing one blocklist file. - Ensure urls from hosts in this blocklist get blocked.""" - blocklist = create_blocklist(tmpdir) - zip_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) - config_stub.data = {'content': - {'host-block-lists': [zip_url], - 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} - host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) - host_blocker._in_progress[0].finished.emit() - host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) + # def test_remote_text(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single fakely remote text blocklist. + # Ensure urls from hosts in this blocklist get blocked.""" + # blocklist = create_blocklist(tmpdir) + # config_stub.data = {'content': + # {'host-block-lists': [blocklist], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # host_blocker.adblock_update(0) + # # Simulate download is finished + # # XXX Is it ok to use private attribute hostblocker._in_progress ? + # host_blocker._in_progress[0].finished.emit() + # host_blocker.read_hosts() + # assert_urls(host_blocker, whitelisted_hosts=[]) - # FIXME adblock.guess_zip_filename should raise FileNotFound Error - # as no files in the zip are called hosts - def test_remote_zip_multi1(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """Update with single fakely remote zip containing two files. - None of them is called hosts, FileNotFoundError should be raised.""" - file1 = create_blocklist(tmpdir, name='file1.txt') - file2_hosts = ['testa.com', 'testb.com'] - file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') - files_to_zip = [file1.path(), file2.path()] - zip_path = create_zipfile(files_to_zip, tmpdir)[1] - zip_url = QUrl(zip_path) - config_stub.data = {'content': - {'host-block-lists': [zip_url], - 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} - host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) - host_blocker._in_progress[0].finished.emit() - host_blocker.read_hosts() - #with pytest.raises(FileNotFoundError): - for str_url in URLS_TO_CHECK: - assert not host_blocker.is_blocked(QUrl(str_url)) + # def test_remote_zip_single(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single fakely remote zip containing one blocklist file. + # Ensure urls from hosts in this blocklist get blocked.""" + # blocklist = create_blocklist(tmpdir) + # zip_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) + # config_stub.data = {'content': + # {'host-block-lists': [zip_url], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # host_blocker.adblock_update(0) + # host_blocker._in_progress[0].finished.emit() + # host_blocker.read_hosts() + # assert_urls(host_blocker, whitelisted_hosts=[]) - def test_remote_zip_multi2(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """Update with single fakely remote zip containing two files. - One of them is called hosts and should be used as blocklist. - Ensure urls from hosts in this blocklist get blocked - and the hosts from the other file are not.""" - file1 = create_blocklist(tmpdir, name='hosts.txt') - file2_hosts = ['testa.com', 'testb.com'] - file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') - files_to_zip = [file1.path(), file2.path()] - zip_path = create_zipfile(files_to_zip, tmpdir)[1] - zip_url = QUrl(zip_path) - config_stub.data = {'content': - {'host-block-lists': [zip_url], - 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} - host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) - host_blocker._in_progress[0].finished.emit() - host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) + # # FIXME adblock.guess_zip_filename should raise FileNotFound Error + # # as no files in the zip are called hosts + # def test_remote_zip_multi1(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single fakely remote zip containing two files. + # None of them is called hosts, FileNotFoundError should be raised.""" + # file1 = create_blocklist(tmpdir, name='file1.txt') + # file2_hosts = ['testa.com', 'testb.com'] + # file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') + # files_to_zip = [file1.path(), file2.path()] + # zip_path = create_zipfile(files_to_zip, tmpdir)[1] + # zip_url = QUrl(zip_path) + # config_stub.data = {'content': + # {'host-block-lists': [zip_url], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # #with pytest.raises(FileNotFoundError): + # host_blocker.adblock_update(0) + # host_blocker._in_progress[0].finished.emit() + # host_blocker.read_hosts() + # for str_url in URLS_TO_CHECK: + # assert not host_blocker.is_blocked(QUrl(str_url)) - def test_local_text(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """Update with single local text blocklist. - Ensure urls from hosts in this blocklist get blocked.""" - blocklist = create_blocklist(tmpdir) - blocklist.setScheme("file") + # def test_remote_zip_multi2(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single fakely remote zip containing two files. + # One of them is called hosts and should be used as blocklist. + # Ensure urls from hosts in this blocklist get blocked + # and the hosts from the other file are not.""" + # file1 = create_blocklist(tmpdir, name='hosts.txt') + # file2_hosts = ['testa.com', 'testb.com'] + # file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') + # files_to_zip = [file1.path(), file2.path()] + # zip_path = create_zipfile(files_to_zip, tmpdir)[1] + # zip_url = QUrl(zip_path) + # config_stub.data = {'content': + # {'host-block-lists': [zip_url], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # host_blocker.adblock_update(0) + # host_blocker._in_progress[0].finished.emit() + # host_blocker.read_hosts() + # assert_urls(host_blocker, whitelisted_hosts=[]) + + # def test_local_text(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single local text blocklist. + # Ensure urls from hosts in this blocklist get blocked.""" + # blocklist = create_blocklist(tmpdir) + # blocklist.setScheme("file") + # config_stub.data = {'content': + # {'host-block-lists': [blocklist], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # host_blocker.adblock_update(0) + # host_blocker.read_hosts() + # assert_urls(host_blocker, whitelisted_hosts=[]) + + # def test_local_zip_single(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single local zip containing one file. + # Ensure urls from hosts in this blocklist get blocked.""" + # blocklist = create_blocklist(tmpdir) + # zip_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) + # zip_url.setScheme('file') + # config_stub.data = {'content': + # {'host-block-lists': [zip_url], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # host_blocker.adblock_update(0) + # host_blocker.read_hosts() + # assert_urls(host_blocker, whitelisted_hosts=[]) + + # def test_local_zip_multi(self, config_stub, basedir, download_stub, + # data_tmpdir, tmpdir, win_registry): + # """Update with single local zip containing two files. + # One of them is called hosts and should be used as blocklist. + # Ensure urls from hosts in this blocklist get blocked + # and the hosts from the other file are not.""" + # file1 = create_blocklist(tmpdir, name='hosts.txt') + # file2_hosts = ['testa.com', 'testb.com'] + # file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') + # files_to_zip = [file1.path(), file2.path()] + # zip_path = create_zipfile(files_to_zip, tmpdir)[1] + # zip_url = QUrl(zip_path) + # zip_url.setScheme('file') + # config_stub.data = {'content': + # {'host-block-lists': [zip_url], + # 'host-blocking-enabled': True, + # 'host-blocking-whitelist': None}} + # host_blocker = adblock.HostBlocker() + # host_blocker.adblock_update(0) + # host_blocker.read_hosts() + # assert_urls(host_blocker, whitelisted_hosts=[]) + +class TestHostBlockerIsBlocked: + + """Tests for function adblock_update of class HostBlocker.""" + + def test_with_whitelist(self, config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + """""" + # Simulate adblock_update has already been run, + # exclude localhost from blocked-hosts file + filtered_blocked_hosts = BLOCKED_HOSTS[:-1] + blocklist = create_blocklist(tmpdir, + blocked_hosts=filtered_blocked_hosts, + name="blocked-hosts") config_stub.data = {'content': {'host-block-lists': [blocklist], 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} + 'host-blocking-whitelist': WHITELISTED_HOSTS}} host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) - - def test_local_zip_multi(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """Update with single local zip containing two files. - One of them is called hosts and should be used as blocklist. - Ensure urls from hosts in this blocklist get blocked - and the hosts from the other file are not.""" - file1 = create_blocklist(tmpdir, name='hosts.txt') - file2_hosts = ['testa.com', 'testb.com'] - file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') - files_to_zip = [file1.path(), file2.path()] - zip_path = create_zipfile(files_to_zip, tmpdir)[1] - zip_url = QUrl(zip_path) - zip_url.setScheme('file') - config_stub.data = {'content': - {'host-block-lists': [zip_url], - 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} - host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) - host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) + assert_urls(host_blocker) From f8fcd48998ce3c2ca843ed6e6e1b30873083a507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Mon, 7 Dec 2015 00:11:19 +0100 Subject: [PATCH 17/21] remove now useless tests --- tests/unit/browser/test_adblock.py | 132 ----------------------------- 1 file changed, 132 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 78019d301..d48c49f22 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -238,138 +238,6 @@ class TestHostBlockerUpdate: host_blocker.read_hosts() assert_urls(host_blocker, BLOCKED_HOSTS[3:], whitelisted_hosts=[]) - # def test_remote_text(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single fakely remote text blocklist. - # Ensure urls from hosts in this blocklist get blocked.""" - # blocklist = create_blocklist(tmpdir) - # config_stub.data = {'content': - # {'host-block-lists': [blocklist], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # host_blocker.adblock_update(0) - # # Simulate download is finished - # # XXX Is it ok to use private attribute hostblocker._in_progress ? - # host_blocker._in_progress[0].finished.emit() - # host_blocker.read_hosts() - # assert_urls(host_blocker, whitelisted_hosts=[]) - - # def test_remote_zip_single(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single fakely remote zip containing one blocklist file. - # Ensure urls from hosts in this blocklist get blocked.""" - # blocklist = create_blocklist(tmpdir) - # zip_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) - # config_stub.data = {'content': - # {'host-block-lists': [zip_url], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # host_blocker.adblock_update(0) - # host_blocker._in_progress[0].finished.emit() - # host_blocker.read_hosts() - # assert_urls(host_blocker, whitelisted_hosts=[]) - - # # FIXME adblock.guess_zip_filename should raise FileNotFound Error - # # as no files in the zip are called hosts - # def test_remote_zip_multi1(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single fakely remote zip containing two files. - # None of them is called hosts, FileNotFoundError should be raised.""" - # file1 = create_blocklist(tmpdir, name='file1.txt') - # file2_hosts = ['testa.com', 'testb.com'] - # file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') - # files_to_zip = [file1.path(), file2.path()] - # zip_path = create_zipfile(files_to_zip, tmpdir)[1] - # zip_url = QUrl(zip_path) - # config_stub.data = {'content': - # {'host-block-lists': [zip_url], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # #with pytest.raises(FileNotFoundError): - # host_blocker.adblock_update(0) - # host_blocker._in_progress[0].finished.emit() - # host_blocker.read_hosts() - # for str_url in URLS_TO_CHECK: - # assert not host_blocker.is_blocked(QUrl(str_url)) - - # def test_remote_zip_multi2(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single fakely remote zip containing two files. - # One of them is called hosts and should be used as blocklist. - # Ensure urls from hosts in this blocklist get blocked - # and the hosts from the other file are not.""" - # file1 = create_blocklist(tmpdir, name='hosts.txt') - # file2_hosts = ['testa.com', 'testb.com'] - # file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') - # files_to_zip = [file1.path(), file2.path()] - # zip_path = create_zipfile(files_to_zip, tmpdir)[1] - # zip_url = QUrl(zip_path) - # config_stub.data = {'content': - # {'host-block-lists': [zip_url], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # host_blocker.adblock_update(0) - # host_blocker._in_progress[0].finished.emit() - # host_blocker.read_hosts() - # assert_urls(host_blocker, whitelisted_hosts=[]) - - # def test_local_text(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single local text blocklist. - # Ensure urls from hosts in this blocklist get blocked.""" - # blocklist = create_blocklist(tmpdir) - # blocklist.setScheme("file") - # config_stub.data = {'content': - # {'host-block-lists': [blocklist], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # host_blocker.adblock_update(0) - # host_blocker.read_hosts() - # assert_urls(host_blocker, whitelisted_hosts=[]) - - # def test_local_zip_single(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single local zip containing one file. - # Ensure urls from hosts in this blocklist get blocked.""" - # blocklist = create_blocklist(tmpdir) - # zip_url = QUrl(create_zipfile([blocklist.path()], tmpdir)[1]) - # zip_url.setScheme('file') - # config_stub.data = {'content': - # {'host-block-lists': [zip_url], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # host_blocker.adblock_update(0) - # host_blocker.read_hosts() - # assert_urls(host_blocker, whitelisted_hosts=[]) - - # def test_local_zip_multi(self, config_stub, basedir, download_stub, - # data_tmpdir, tmpdir, win_registry): - # """Update with single local zip containing two files. - # One of them is called hosts and should be used as blocklist. - # Ensure urls from hosts in this blocklist get blocked - # and the hosts from the other file are not.""" - # file1 = create_blocklist(tmpdir, name='hosts.txt') - # file2_hosts = ['testa.com', 'testb.com'] - # file2 = create_blocklist(tmpdir, file2_hosts, name='file2.txt') - # files_to_zip = [file1.path(), file2.path()] - # zip_path = create_zipfile(files_to_zip, tmpdir)[1] - # zip_url = QUrl(zip_path) - # zip_url.setScheme('file') - # config_stub.data = {'content': - # {'host-block-lists': [zip_url], - # 'host-blocking-enabled': True, - # 'host-blocking-whitelist': None}} - # host_blocker = adblock.HostBlocker() - # host_blocker.adblock_update(0) - # host_blocker.read_hosts() - # assert_urls(host_blocker, whitelisted_hosts=[]) - class TestHostBlockerIsBlocked: """Tests for function adblock_update of class HostBlocker.""" From 99ede3f551207e733d343a25d8d890b90ef65c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Tue, 8 Dec 2015 00:52:51 +0100 Subject: [PATCH 18/21] Improve coverage and code cleanup --- tests/unit/browser/test_adblock.py | 418 ++++++++++++++++++++--------- 1 file changed, 295 insertions(+), 123 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index d48c49f22..5336b4ee0 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -32,45 +32,52 @@ from qutebrowser.browser import adblock from qutebrowser.utils import objreg from qutebrowser.commands import cmdexc -WHITELISTED_HOSTS = ('qutebrowser.org', 'goodhost.gov') -BLOCKED_HOSTS = ('verygoodhost.com', - 'goodhost.gov', - 'mediumhost.io', - 'malware.badhost.org', - '4-verybadhost.com', - 'ads.worsthostever.net', - 'localhost') +WHITELISTED_HOSTS = ('qutebrowser.org', 'mediumhost.io') + +BLOCKLIST_HOSTS = ('localhost', + 'mediumhost.io', + 'malware.badhost.org', + '4-verybadhost.com', + 'ads.worsthostever.net') + +CLEAN_HOSTS = ('goodhost.gov', 'verygoodhost.com') URLS_TO_CHECK = ('http://localhost', - 'ftp://goodhost.gov', 'http://mediumhost.io', 'http://malware.badhost.org', 'http://4-verybadhost.com', 'http://ads.worsthostever.net', - 'http://verygoodhost.com', - 'ftp://perfecthost.com', + 'http://goodhost.gov', + 'ftp://verygoodhost.com' 'http://qutebrowser.org') + @pytest.fixture def data_tmpdir(monkeypatch, tmpdir): + """Set tmpdir as datadir""" - monkeypatch.setattr('qutebrowser.utils.standarddir.data', - lambda: str(tmpdir)) + + tmpdir = str(tmpdir) + monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: tmpdir) # XXX Why does read_hosts needs basedir to be None # in order to print message 'run adblock-update to read host blocklist' ? # browser/adblock.py line 138 class BaseDirStub: - """Mock for objreg.get['args'] - called in adblock.HostBlocker.read_hosts.""" + + """Mock for objreg.get('args') called in adblock.HostBlocker.read_hosts.""" + def __init__(self): self.basedir = None + @pytest.yield_fixture def basedir(): + """Register a Fake basedir.""" + args = BaseDirStub() objreg.register('args', args) yield args @@ -78,22 +85,29 @@ def basedir(): class FakeDownloadItem(QObject): + """Mock browser.downloads.DownloadItem.""" finished = pyqtSignal() - def __init__(self, fileobj): + def __init__(self, fileobj, name): super().__init__() self.fileobj = fileobj + self.name = name self.successful = True + class FakeDownloadManager: + """Mock browser.downloads.DownloadManager.""" def get(self, url, fileobj, **kwargs): - """Returns a FakeDownloadItem instance with a fileobj - copied from given fake url file.""" - download_item = FakeDownloadItem(fileobj) + + """Return a FakeDownloadItem instance with a fileobj + whose content is copied from the file the given url links to. + """ + + download_item = FakeDownloadItem(fileobj, name=url.path()) with open(url.path(), 'rb') as fake_url_file: # Ensure cursors are at position 0 before copying fake_url_file.seek(0) @@ -101,9 +115,12 @@ class FakeDownloadManager: shutil.copyfileobj(fake_url_file, download_item.fileobj) return download_item + @pytest.yield_fixture def download_stub(win_registry): + """Register a FakeDownloadManager.""" + stub = FakeDownloadManager() objreg.register('download-manager', stub, scope='window', window='last-focused') @@ -111,150 +128,305 @@ def download_stub(win_registry): objreg.delete('download-manager', scope='window', window='last-focused') +def create_zipfile(directory, files, zipname='test'): + + """Return a path to a created zip file + + Args : + - directory : path object where to create the zip file + - files : list of paths to each file to add in the zipfile + - zipname : name to give to the zip file. + """ -def create_zipfile(files, directory, zipname='test'): - """Returns a zipfile populated with given files and its name.""" directory = str(directory) zipfile_path = os.path.join(directory, zipname + '.zip') with zipfile.ZipFile(zipfile_path, 'w') as new_zipfile: for file_name in files: new_zipfile.write(file_name, arcname=os.path.basename(file_name)) # Removes path from file name - return new_zipfile, zipfile_path + return zipfile_path + + +def create_blocklist(directory, + blocked_hosts=BLOCKLIST_HOSTS, + name='hosts', + line_format='one_per_line'): + + """Return a QUrl instance linking to a blocklist file. + + Args: + - directory : path object where to create the blocklist file + - blocked_hosts : an iterable of string hosts to add to the blocklist + - name : name to give to the blocklist file + - line_format : 'etc_hosts' --> /etc/hosts format + 'one_per_line' --> one host per line format + 'not_correct' --> Not a correct hosts file format. + """ -def create_blocklist(directory, blocked_hosts=BLOCKED_HOSTS, - name='hosts', line_format=None): - """Returns a QUrl instance linking to a file with given name in given - directory which contains a list of given blocked_hosts.""" blocklist = QUrl(os.path.join(str(directory), name)) with open(blocklist.path(), 'w', encoding='UTF-8') as hosts: + # ensure comments are ignored when processing blocklist hosts.write('# Blocked Hosts List #\n\n') - if line_format == 'etc_hosts': + if line_format == 'etc_hosts': # /etc/hosts like format for path in blocked_hosts: hosts.write('127.0.0.1 ' + path + '\n') - elif line_format == 'no_hosts': - hosts.write('This file is not a hosts file') - else: #one host per line format + elif line_format == 'one_per_line': for path in blocked_hosts: hosts.write(path + '\n') + elif line_format == 'not_correct': + for path in blocked_hosts: + hosts.write(path + ' This is not a correct hosts file\n') return blocklist + def assert_urls(host_blocker, - blocked_hosts=BLOCKED_HOSTS, + blocked=BLOCKLIST_HOSTS, whitelisted_hosts=WHITELISTED_HOSTS, urls_to_check=URLS_TO_CHECK): + """Test if urls_to_check are effectively blocked or not by HostBlocker - Url in blocked_hosts and not in whitelisted_hosts should be blocked - All other Urls should not be blocked.""" - whitelisted_hosts = list(whitelisted_hosts) + list(host_blocker.WHITELISTED) + Ensure Urls in blocked and not in whitelisted are blocked + All other Urls are not to be blocked.""" + + whitelisted = list(whitelisted_hosts) + list(host_blocker.WHITELISTED) for str_url in urls_to_check: url = QUrl(str_url) host = url.host() - if host in blocked_hosts and host not in whitelisted_hosts: + if host in blocked and host not in whitelisted: assert host_blocker.is_blocked(url) else: assert not host_blocker.is_blocked(url) -class TestHostBlockerUpdate: +def generic_blocklists(directory): - """Tests for function adblock_update of class HostBlocker.""" + """Return a generic list of files to be used in hosts-block-lists option. + This list contains : + - a remote zip file with 1 hosts file and 2 useless files + - a remote zip file with only useless files + (Should raise a FileNotFoundError) + - a remote zip file with only one valid hosts file + - a local text file with valid hosts + - a remote text file without valid hosts format.""" - def generic_blocklists(self, directory): - file1 = create_blocklist(directory, BLOCKED_HOSTS[5:], 'hosts', 'etc_hosts') - file2 = create_blocklist(directory, name='README', line_format='no_hosts') - file3 = create_blocklist(directory, BLOCKED_HOSTS[:2], 'false_positive') - files_to_zip = [file1.path(), file2.path(), file3.path()] - zip_path = create_zipfile(files_to_zip, directory, 'block1')[1] - remote_blocklist1 = QUrl(zip_path) + # remote zip file with 1 hosts file and 2 useless files + file1 = create_blocklist(directory, + blocked_hosts=CLEAN_HOSTS, + name='README', + line_format='not_correct') + file2 = create_blocklist(directory, + blocked_hosts=BLOCKLIST_HOSTS[:3], + name='hosts', + line_format='etc_hosts') + file3 = create_blocklist(directory, + blocked_hosts=CLEAN_HOSTS, + name='false_positive', + line_format='one_per_line') + files_to_zip = [file1.path(), file2.path(), file3.path()] + zip_path = create_zipfile(directory, files_to_zip, 'block1') + blocklist1 = QUrl(zip_path) - blocklist2 = create_blocklist(directory, [BLOCKED_HOSTS[4]], 'malwarelist', 'etc_hosts') - zip_path = create_zipfile([blocklist2.path()], directory, 'block2')[1] - remote_blocklist2 = QUrl(zip_path) + # remote zip file without file named hosts + # (Should raise a FileNotFoundError) + file1 = create_blocklist(directory, + blocked_hosts=CLEAN_HOSTS, + name='md5sum', + line_format='etc_hosts') + file2 = create_blocklist(directory, + blocked_hosts=CLEAN_HOSTS, + name='README', + line_format='not_correct') + file3 = create_blocklist(directory, + blocked_hosts=CLEAN_HOSTS, + name='false_positive', + line_format='one_per_line') + files_to_zip = [file1.path(), file2.path(), file3.path()] + zip_path = create_zipfile(directory, files_to_zip, 'block2') + blocklist2 = QUrl(zip_path) - # A local list with one host per line - local_blocklist3 = create_blocklist(directory, [BLOCKED_HOSTS[3]], 'mycustomblocklist', 'etc_hosts') - local_blocklist3.setScheme('file') + # remote zip file with only one valid hosts file inside + blocklist3 = create_blocklist(directory, + blocked_hosts=[BLOCKLIST_HOSTS[3]], + name='malwarelist', + line_format='etc_hosts') + zip_path = create_zipfile(directory, [blocklist3.path()], 'block3') + blocklist3 = QUrl(zip_path) - # A list that cannot be read due to its formatting - remote_blocklist4 = create_blocklist(directory, [BLOCKED_HOSTS[2]], 'badlist', 'no_hosts') + # local text file with valid hosts + blocklist4 = create_blocklist(directory, + blocked_hosts=[BLOCKLIST_HOSTS[4]], + name='mycustomblocklist', + line_format='one_per_line') + blocklist4.setScheme('file') - return [remote_blocklist1, remote_blocklist2, local_blocklist3, remote_blocklist4] + # remote text file without valid hosts format + blocklist5 = create_blocklist(directory, + blocked_hosts=CLEAN_HOSTS, + name='notcorrectlist', + line_format='not_correct') - def test_without_datadir(self, config_stub, tmpdir, - monkeypatch, win_registry): - """No directory for data configured so no hosts file exists. - CommandError is raised by adblock_update - Ensure no url is blocked.""" - config_stub.data = {'content': - {'host-block-lists': self.generic_blocklists(tmpdir), - 'host-blocking-enabled': True}} - monkeypatch.setattr('qutebrowser.utils.standarddir.data', - lambda: None) - host_blocker = adblock.HostBlocker() - with pytest.raises(cmdexc.CommandError): - host_blocker.adblock_update(0) - host_blocker.read_hosts() - for str_url in URLS_TO_CHECK: - assert not host_blocker.is_blocked(QUrl(str_url)) + return [blocklist1, blocklist2, blocklist3, blocklist4, blocklist5] - def test_disabled_blocking(self, basedir, config_stub, download_stub, - data_tmpdir, tmpdir, win_registry): - """Ensure that no url is blocked when host blocking is disabled.""" - config_stub.data = {'content': - {'host-block-lists': self.generic_blocklists(tmpdir), - 'host-blocking-enabled': False}} - host_blocker = adblock.HostBlocker() + +def test_without_datadir(config_stub, tmpdir, monkeypatch, win_registry): + + """No directory for data configured so no hosts file exists. + CommandError is raised by adblock_update + Ensure no url is blocked.""" + + config_stub.data = {'content': + {'host-block-lists': generic_blocklists(tmpdir), + 'host-blocking-enabled': True}} + monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: None) + host_blocker = adblock.HostBlocker() + with pytest.raises(cmdexc.CommandError): host_blocker.adblock_update(0) - host_blocker._in_progress[0].finished.emit() - host_blocker.read_hosts() - for str_url in URLS_TO_CHECK: - assert not host_blocker.is_blocked(QUrl(str_url)) + host_blocker.read_hosts() + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) - def test_no_blocklist(self, config_stub, download_stub, - data_tmpdir, basedir, tmpdir, win_registry): - """Ensure no host is blocked when no blocklist exists.""" - config_stub.data = {'content': - {'host-block-lists' : None, - 'host-blocking-enabled': True}} - host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) - host_blocker.read_hosts() - for str_url in URLS_TO_CHECK: - assert not host_blocker.is_blocked(QUrl(str_url)) - def test_successful_update(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - config_stub.data = {'content': - {'host-block-lists': self.generic_blocklists(tmpdir), - 'host-blocking-enabled': True, - 'host-blocking-whitelist': None}} - host_blocker = adblock.HostBlocker() - host_blocker.adblock_update(0) - # Simulate download is finished - # XXX Is it ok to use private attribute hostblocker._in_progress ? - while host_blocker._in_progress != []: - host_blocker._in_progress[0].finished.emit() - host_blocker.read_hosts() - assert_urls(host_blocker, BLOCKED_HOSTS[3:], whitelisted_hosts=[]) +def test_disabled_blocking_update(basedir, config_stub, download_stub, + data_tmpdir, tmpdir, win_registry): -class TestHostBlockerIsBlocked: + """Ensure that no url is blocked when host blocking is disabled.""" - """Tests for function adblock_update of class HostBlocker.""" + config_stub.data = {'content': + {'host-block-lists': generic_blocklists(tmpdir), + 'host-blocking-enabled': False}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + while host_blocker._in_progress: + current_download = host_blocker._in_progress[0] + current_download.finished.emit() + host_blocker.read_hosts() + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) - def test_with_whitelist(self, config_stub, basedir, download_stub, - data_tmpdir, tmpdir, win_registry): - """""" - # Simulate adblock_update has already been run, - # exclude localhost from blocked-hosts file - filtered_blocked_hosts = BLOCKED_HOSTS[:-1] - blocklist = create_blocklist(tmpdir, - blocked_hosts=filtered_blocked_hosts, - name="blocked-hosts") - config_stub.data = {'content': - {'host-block-lists': [blocklist], - 'host-blocking-enabled': True, - 'host-blocking-whitelist': WHITELISTED_HOSTS}} - host_blocker = adblock.HostBlocker() - host_blocker.read_hosts() - assert_urls(host_blocker) + +def test_no_blocklist_update(config_stub, download_stub, + data_tmpdir, basedir, tmpdir, win_registry): + + """Ensure no host is blocked when no blocklist exists.""" + + config_stub.data = {'content': + {'host-block-lists' : None, + 'host-blocking-enabled': True}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + host_blocker.read_hosts() + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) + + +def test_successful_update(config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + + """Ensure hosts from host-block-lists are correctly + blocked after update.""" + + config_stub.data = {'content': + {'host-block-lists': generic_blocklists(tmpdir), + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + # Simulate download is finished + # XXX Is it ok to use private list hostblocker._in_progress ? + while host_blocker._in_progress: + current_download = host_blocker._in_progress[0] + current_download.finished.emit() + host_blocker.read_hosts() + assert_urls(host_blocker, whitelisted_hosts=[]) + + +def test_failed_dl_update(config_stub, basedir, download_stub, + data_tmpdir, tmpdir, win_registry): + + """ One blocklist fails to download + Ensure no hosts from this list is blocked.""" + + dl_fail_blocklist = create_blocklist(tmpdir, + blocked_hosts=CLEAN_HOSTS, + name='download_will_fail', + line_format='one_per_line') + hosts_to_block = generic_blocklists(tmpdir) + [dl_fail_blocklist] + config_stub.data = {'content': + {'host-block-lists': hosts_to_block, + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.adblock_update(0) + while host_blocker._in_progress: + current_download = host_blocker._in_progress[0] + # if current download is the file we want to fail, make it fail + if current_download.name == dl_fail_blocklist.path(): + current_download.successful = False + current_download.finished.emit() + host_blocker.read_hosts() + assert_urls(host_blocker, whitelisted_hosts=[]) + + +def test_blocking_with_whitelist(config_stub, basedir, download_stub, + data_tmpdir, tmpdir): + + """Ensure hosts in host-blocking-whitelist are taken into account.""" + + # Simulate adblock_update has already been run by naming it blocked-hosts, + # exclude localhost from blocked-hosts file as it is in + # host_blocker.WHITELISTED + filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] + blocklist = create_blocklist(tmpdir, + blocked_hosts=filtered_blocked_hosts, + name='blocked-hosts', + line_format='one_per_line') + config_stub.data = {'content': + {'host-block-lists': [blocklist], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': WHITELISTED_HOSTS}} + host_blocker = adblock.HostBlocker() + host_blocker.read_hosts() + assert_urls(host_blocker) + + +# XXX Intended behavior ? +# User runs qutebrowser with host-blocking enabled +# A blocklist is present in host-block-lists and blocked_hosts is populated +# +# User quits qutebrowser, empties host-block-lists from his config +# User restarts qutebrowser, does adblock-update (which will return None) +# read_hosts still returns hosts from unchanged blocked_hosts file +# +# Is this intended behavior or shouldn't on_config_changed be also called +# during HostBlocker instance init to avoid this ? +# +# As a comparison, +# host-block-lists is emptied with qutebrowser running +# on_config_changed slot is activated +# blocked_hosts is emptied +# read_hosts doesn't return any host as expected + +def test_config_change(config_stub, basedir, download_stub, + data_tmpdir, tmpdir): + + """Ensure blocked_hosts gets a reset when host-block-list + is changed to None.""" + + # Simulate adblock_update has already been run by naming it blocked-hosts, + # exclude localhost from blocked-hosts file as it is in + # host_blocker.WHITELISTED + filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] + blocklist = create_blocklist(tmpdir, + blocked_hosts=filtered_blocked_hosts, + name='blocked-hosts', + line_format='one_per_line') + config_stub.data = {'content': + {'host-block-lists': [blocklist], + 'host-blocking-enabled': True, + 'host-blocking-whitelist': None}} + host_blocker = adblock.HostBlocker() + host_blocker.read_hosts() + config_stub.set('content', 'host-block-lists', None) + host_blocker.read_hosts() + for str_url in URLS_TO_CHECK: + assert not host_blocker.is_blocked(QUrl(str_url)) From ff8d5370a33b744420a27fc5263220530e747e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Tue, 8 Dec 2015 10:37:04 +0100 Subject: [PATCH 19/21] Add ideas for improvement --- tests/unit/browser/test_adblock.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 5336b4ee0..a670756cd 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -1,3 +1,4 @@ +# TODO See utils/test_standarddirutils for OSEError and caplog assertion # vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: #!/usr/bin/env python3 @@ -65,6 +66,7 @@ def data_tmpdir(monkeypatch, tmpdir): # XXX Why does read_hosts needs basedir to be None # in order to print message 'run adblock-update to read host blocklist' ? # browser/adblock.py line 138 +# TODO Replace with unittest mock.mock / See test webelem class BaseDirStub: """Mock for objreg.get('args') called in adblock.HostBlocker.read_hosts.""" From 47261dbd3054cc4a3b482690d073b63b4b2a6f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sat, 23 Jan 2016 16:15:19 +0100 Subject: [PATCH 20/21] Code cleanup / Good practices --- tests/unit/browser/test_adblock.py | 221 +++++++++++------------------ 1 file changed, 79 insertions(+), 142 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index a670756cd..9fdce5446 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -1,8 +1,7 @@ -# TODO See utils/test_standarddirutils for OSEError and caplog assertion # vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: #!/usr/bin/env python3 -# Copyright 2014-2015 Florian Bruhin (The Compiler) +# Copyright 2015 Corentin Julé # # This file is part of qutebrowser. # @@ -33,6 +32,7 @@ from qutebrowser.browser import adblock from qutebrowser.utils import objreg from qutebrowser.commands import cmdexc +# TODO See ../utils/test_standarddirutils for OSEError and caplog assertion WHITELISTED_HOSTS = ('qutebrowser.org', 'mediumhost.io') @@ -46,27 +46,20 @@ CLEAN_HOSTS = ('goodhost.gov', 'verygoodhost.com') URLS_TO_CHECK = ('http://localhost', 'http://mediumhost.io', - 'http://malware.badhost.org', + 'ftp://malware.badhost.org', 'http://4-verybadhost.com', 'http://ads.worsthostever.net', 'http://goodhost.gov', 'ftp://verygoodhost.com' 'http://qutebrowser.org') - @pytest.fixture def data_tmpdir(monkeypatch, tmpdir): - """Set tmpdir as datadir""" - tmpdir = str(tmpdir) monkeypatch.setattr('qutebrowser.utils.standarddir.data', lambda: tmpdir) -# XXX Why does read_hosts needs basedir to be None -# in order to print message 'run adblock-update to read host blocklist' ? -# browser/adblock.py line 138 -# TODO Replace with unittest mock.mock / See test webelem class BaseDirStub: """Mock for objreg.get('args') called in adblock.HostBlocker.read_hosts.""" @@ -77,12 +70,10 @@ class BaseDirStub: @pytest.yield_fixture def basedir(): - """Register a Fake basedir.""" - args = BaseDirStub() objreg.register('args', args) - yield args + yield objreg.delete('args') @@ -92,8 +83,8 @@ class FakeDownloadItem(QObject): finished = pyqtSignal() - def __init__(self, fileobj, name): - super().__init__() + def __init__(self, fileobj, name, parent=None): + super().__init__(parent) self.fileobj = fileobj self.name = name self.successful = True @@ -104,57 +95,45 @@ class FakeDownloadManager: """Mock browser.downloads.DownloadManager.""" def get(self, url, fileobj, **kwargs): - """Return a FakeDownloadItem instance with a fileobj whose content is copied from the file the given url links to. """ - download_item = FakeDownloadItem(fileobj, name=url.path()) with open(url.path(), 'rb') as fake_url_file: - # Ensure cursors are at position 0 before copying - fake_url_file.seek(0) - download_item.fileobj.seek(0) shutil.copyfileobj(fake_url_file, download_item.fileobj) return download_item @pytest.yield_fixture def download_stub(win_registry): - """Register a FakeDownloadManager.""" - stub = FakeDownloadManager() objreg.register('download-manager', stub, scope='window', window='last-focused') - yield stub + yield objreg.delete('download-manager', scope='window', window='last-focused') def create_zipfile(directory, files, zipname='test'): - - """Return a path to a created zip file + """Return a path to a newly created zip file Args : - directory : path object where to create the zip file - files : list of paths to each file to add in the zipfile - zipname : name to give to the zip file. """ - - directory = str(directory) - zipfile_path = os.path.join(directory, zipname + '.zip') - with zipfile.ZipFile(zipfile_path, 'w') as new_zipfile: - for file_name in files: - new_zipfile.write(file_name, arcname=os.path.basename(file_name)) + zipfile_path = directory / zipname + '.zip' + with zipfile.ZipFile(str(zipfile_path), 'w') as new_zipfile: + for file_path in files: + new_zipfile.write(str(file_path), + arcname=os.path.basename(str(file_path))) # Removes path from file name - return zipfile_path + return str(zipfile_path) -def create_blocklist(directory, - blocked_hosts=BLOCKLIST_HOSTS, - name='hosts', - line_format='one_per_line'): - - """Return a QUrl instance linking to a blocklist file. +def create_blocklist(directory, blocked_hosts=BLOCKLIST_HOSTS, + name='hosts', line_format='one_per_line'): + """Return a path to a blocklist file. Args: - directory : path object where to create the blocklist file @@ -164,33 +143,30 @@ def create_blocklist(directory, 'one_per_line' --> one host per line format 'not_correct' --> Not a correct hosts file format. """ - - blocklist = QUrl(os.path.join(str(directory), name)) - with open(blocklist.path(), 'w', encoding='UTF-8') as hosts: + blocklist_file = directory / name + with open(str(blocklist_file), 'w', encoding='UTF-8') as blocklist: # ensure comments are ignored when processing blocklist - hosts.write('# Blocked Hosts List #\n\n') + blocklist.write('# Blocked Hosts List #\n\n') if line_format == 'etc_hosts': # /etc/hosts like format - for path in blocked_hosts: - hosts.write('127.0.0.1 ' + path + '\n') + for host in blocked_hosts: + blocklist.write('127.0.0.1 ' + host + '\n') elif line_format == 'one_per_line': - for path in blocked_hosts: - hosts.write(path + '\n') + for host in blocked_hosts: + blocklist.write(host + '\n') elif line_format == 'not_correct': - for path in blocked_hosts: - hosts.write(path + ' This is not a correct hosts file\n') - return blocklist + for host in blocked_hosts: + blocklist.write(host + ' This is not a correct hosts file\n') + else: + raise ValueError('Uncorrect line_format argument') + return str(blocklist_file) -def assert_urls(host_blocker, - blocked=BLOCKLIST_HOSTS, - whitelisted_hosts=WHITELISTED_HOSTS, - urls_to_check=URLS_TO_CHECK): - - """Test if urls_to_check are effectively blocked or not by HostBlocker +def assert_urls(host_blocker, blocked=BLOCKLIST_HOSTS, + whitelisted=WHITELISTED_HOSTS, urls_to_check=URLS_TO_CHECK): + """Test if Urls to check are blocked or not by HostBlocker Ensure Urls in blocked and not in whitelisted are blocked - All other Urls are not to be blocked.""" - - whitelisted = list(whitelisted_hosts) + list(host_blocker.WHITELISTED) + All other Urls must not be blocked.""" + whitelisted = list(whitelisted) + list(host_blocker.WHITELISTED) for str_url in urls_to_check: url = QUrl(str_url) host = url.host() @@ -201,7 +177,6 @@ def assert_urls(host_blocker, def generic_blocklists(directory): - """Return a generic list of files to be used in hosts-block-lists option. This list contains : - a remote zip file with 1 hosts file and 2 useless files @@ -212,70 +187,50 @@ def generic_blocklists(directory): - a remote text file without valid hosts format.""" # remote zip file with 1 hosts file and 2 useless files - file1 = create_blocklist(directory, - blocked_hosts=CLEAN_HOSTS, - name='README', - line_format='not_correct') - file2 = create_blocklist(directory, - blocked_hosts=BLOCKLIST_HOSTS[:3], - name='hosts', - line_format='etc_hosts') - file3 = create_blocklist(directory, - blocked_hosts=CLEAN_HOSTS, - name='false_positive', - line_format='one_per_line') - files_to_zip = [file1.path(), file2.path(), file3.path()] - zip_path = create_zipfile(directory, files_to_zip, 'block1') - blocklist1 = QUrl(zip_path) + file1 = create_blocklist(directory, blocked_hosts=CLEAN_HOSTS, + name='README', line_format='not_correct') + file2 = create_blocklist(directory, blocked_hosts=BLOCKLIST_HOSTS[:3], + name='hosts', line_format='etc_hosts') + file3 = create_blocklist(directory, blocked_hosts=CLEAN_HOSTS, + name='false_positive', line_format='one_per_line') + files_to_zip = [file1, file2, file3] + blocklist1 = QUrl(create_zipfile(directory, files_to_zip, 'block1')) # remote zip file without file named hosts # (Should raise a FileNotFoundError) - file1 = create_blocklist(directory, - blocked_hosts=CLEAN_HOSTS, - name='md5sum', - line_format='etc_hosts') - file2 = create_blocklist(directory, - blocked_hosts=CLEAN_HOSTS, - name='README', - line_format='not_correct') - file3 = create_blocklist(directory, - blocked_hosts=CLEAN_HOSTS, - name='false_positive', - line_format='one_per_line') - files_to_zip = [file1.path(), file2.path(), file3.path()] - zip_path = create_zipfile(directory, files_to_zip, 'block2') - blocklist2 = QUrl(zip_path) + file1 = create_blocklist(directory, blocked_hosts=CLEAN_HOSTS, + name='md5sum', line_format='etc_hosts') + file2 = create_blocklist(directory, blocked_hosts=CLEAN_HOSTS, + name='README', line_format='not_correct') + file3 = create_blocklist(directory, blocked_hosts=CLEAN_HOSTS, + name='false_positive', line_format='one_per_line') + files_to_zip = [file1, file2, file3] + blocklist2 = QUrl(create_zipfile(directory, files_to_zip, 'block2')) # remote zip file with only one valid hosts file inside blocklist3 = create_blocklist(directory, blocked_hosts=[BLOCKLIST_HOSTS[3]], - name='malwarelist', - line_format='etc_hosts') - zip_path = create_zipfile(directory, [blocklist3.path()], 'block3') - blocklist3 = QUrl(zip_path) + name='malwarelist', line_format='etc_hosts') + blocklist3 = QUrl(create_zipfile(directory, [blocklist3], 'block3')) # local text file with valid hosts - blocklist4 = create_blocklist(directory, - blocked_hosts=[BLOCKLIST_HOSTS[4]], - name='mycustomblocklist', - line_format='one_per_line') + blocklist4 = QUrl(create_blocklist(directory, + blocked_hosts=[BLOCKLIST_HOSTS[4]], + name='mycustomblocklist', + line_format='one_per_line')) blocklist4.setScheme('file') # remote text file without valid hosts format - blocklist5 = create_blocklist(directory, - blocked_hosts=CLEAN_HOSTS, - name='notcorrectlist', - line_format='not_correct') + blocklist5 = QUrl(create_blocklist(directory, blocked_hosts=CLEAN_HOSTS, + name='notcorrectlist', + line_format='not_correct')) return [blocklist1, blocklist2, blocklist3, blocklist4, blocklist5] def test_without_datadir(config_stub, tmpdir, monkeypatch, win_registry): - """No directory for data configured so no hosts file exists. - CommandError is raised by adblock_update - Ensure no url is blocked.""" - + Ensure CommandError is raised and no Url is blocked.""" config_stub.data = {'content': {'host-block-lists': generic_blocklists(tmpdir), 'host-blocking-enabled': True}} @@ -290,9 +245,7 @@ def test_without_datadir(config_stub, tmpdir, monkeypatch, win_registry): def test_disabled_blocking_update(basedir, config_stub, download_stub, data_tmpdir, tmpdir, win_registry): - - """Ensure that no url is blocked when host blocking is disabled.""" - + """Ensure no Url is blocked when host blocking is disabled.""" config_stub.data = {'content': {'host-block-lists': generic_blocklists(tmpdir), 'host-blocking-enabled': False}} @@ -308,9 +261,7 @@ def test_disabled_blocking_update(basedir, config_stub, download_stub, def test_no_blocklist_update(config_stub, download_stub, data_tmpdir, basedir, tmpdir, win_registry): - - """Ensure no host is blocked when no blocklist exists.""" - + """Ensure no Url is blocked when no block list exists.""" config_stub.data = {'content': {'host-block-lists' : None, 'host-blocking-enabled': True}} @@ -323,10 +274,7 @@ def test_no_blocklist_update(config_stub, download_stub, def test_successful_update(config_stub, basedir, download_stub, data_tmpdir, tmpdir, win_registry): - - """Ensure hosts from host-block-lists are correctly - blocked after update.""" - + """Ensure hosts from host-block-lists are blocked after an update.""" config_stub.data = {'content': {'host-block-lists': generic_blocklists(tmpdir), 'host-blocking-enabled': True, @@ -334,24 +282,21 @@ def test_successful_update(config_stub, basedir, download_stub, host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) # Simulate download is finished - # XXX Is it ok to use private list hostblocker._in_progress ? while host_blocker._in_progress: current_download = host_blocker._in_progress[0] current_download.finished.emit() host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) + assert_urls(host_blocker, whitelisted=[]) def test_failed_dl_update(config_stub, basedir, download_stub, data_tmpdir, tmpdir, win_registry): - - """ One blocklist fails to download - Ensure no hosts from this list is blocked.""" - - dl_fail_blocklist = create_blocklist(tmpdir, - blocked_hosts=CLEAN_HOSTS, - name='download_will_fail', - line_format='one_per_line') + """ One blocklist fails to download. + Ensure hosts from this list are not blocked.""" + dl_fail_blocklist = QUrl(create_blocklist(tmpdir, + blocked_hosts=CLEAN_HOSTS, + name='download_will_fail', + line_format='one_per_line')) hosts_to_block = generic_blocklists(tmpdir) + [dl_fail_blocklist] config_stub.data = {'content': {'host-block-lists': hosts_to_block, @@ -366,17 +311,15 @@ def test_failed_dl_update(config_stub, basedir, download_stub, current_download.successful = False current_download.finished.emit() host_blocker.read_hosts() - assert_urls(host_blocker, whitelisted_hosts=[]) + assert_urls(host_blocker, whitelisted=[]) def test_blocking_with_whitelist(config_stub, basedir, download_stub, data_tmpdir, tmpdir): - - """Ensure hosts in host-blocking-whitelist are taken into account.""" - - # Simulate adblock_update has already been run by naming it blocked-hosts, - # exclude localhost from blocked-hosts file as it is in - # host_blocker.WHITELISTED + """Ensure hosts in host-blocking-whitelist are never blocked.""" + # Simulate adblock_update has already been run + # by creating a file named blocked-hosts, + # Exclude localhost from it, since localhost is in HostBlocker.WHITELISTED filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] blocklist = create_blocklist(tmpdir, blocked_hosts=filtered_blocked_hosts, @@ -406,24 +349,18 @@ def test_blocking_with_whitelist(config_stub, basedir, download_stub, # host-block-lists is emptied with qutebrowser running # on_config_changed slot is activated # blocked_hosts is emptied -# read_hosts doesn't return any host as expected +# read_hosts doesn't return any host, as expected def test_config_change(config_stub, basedir, download_stub, data_tmpdir, tmpdir): - - """Ensure blocked_hosts gets a reset when host-block-list - is changed to None.""" - - # Simulate adblock_update has already been run by naming it blocked-hosts, - # exclude localhost from blocked-hosts file as it is in - # host_blocker.WHITELISTED - filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] + """Ensure blocked-hosts resets if host-block-list is changed to None.""" + filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] # Exclude localhost blocklist = create_blocklist(tmpdir, blocked_hosts=filtered_blocked_hosts, name='blocked-hosts', line_format='one_per_line') config_stub.data = {'content': - {'host-block-lists': [blocklist], + {'host-block-lists': [], 'host-blocking-enabled': True, 'host-blocking-whitelist': None}} host_blocker = adblock.HostBlocker() From 6fd8dc4e57c6e0b2fee38f41cef71b14c8489864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20Jul=C3=A9?= Date: Sat, 23 Jan 2016 16:34:05 +0100 Subject: [PATCH 21/21] Correct test_config_change --- tests/unit/browser/test_adblock.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index 9fdce5446..085374472 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -50,9 +50,10 @@ URLS_TO_CHECK = ('http://localhost', 'http://4-verybadhost.com', 'http://ads.worsthostever.net', 'http://goodhost.gov', - 'ftp://verygoodhost.com' + 'ftp://verygoodhost.com', 'http://qutebrowser.org') + @pytest.fixture def data_tmpdir(monkeypatch, tmpdir): """Set tmpdir as datadir""" @@ -147,7 +148,7 @@ def create_blocklist(directory, blocked_hosts=BLOCKLIST_HOSTS, with open(str(blocklist_file), 'w', encoding='UTF-8') as blocklist: # ensure comments are ignored when processing blocklist blocklist.write('# Blocked Hosts List #\n\n') - if line_format == 'etc_hosts': # /etc/hosts like format + if line_format == 'etc_hosts': # /etc/hosts like format for host in blocked_hosts: blocklist.write('127.0.0.1 ' + host + '\n') elif line_format == 'one_per_line': @@ -263,7 +264,7 @@ def test_no_blocklist_update(config_stub, download_stub, data_tmpdir, basedir, tmpdir, win_registry): """Ensure no Url is blocked when no block list exists.""" config_stub.data = {'content': - {'host-block-lists' : None, + {'host-block-lists': None, 'host-blocking-enabled': True}} host_blocker = adblock.HostBlocker() host_blocker.adblock_update(0) @@ -354,13 +355,13 @@ def test_blocking_with_whitelist(config_stub, basedir, download_stub, def test_config_change(config_stub, basedir, download_stub, data_tmpdir, tmpdir): """Ensure blocked-hosts resets if host-block-list is changed to None.""" - filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] # Exclude localhost - blocklist = create_blocklist(tmpdir, - blocked_hosts=filtered_blocked_hosts, - name='blocked-hosts', - line_format='one_per_line') + filtered_blocked_hosts = BLOCKLIST_HOSTS[1:] # Exclude localhost + blocklist = QUrl(create_blocklist(tmpdir, + blocked_hosts=filtered_blocked_hosts, + name='blocked-hosts', + line_format='one_per_line')) config_stub.data = {'content': - {'host-block-lists': [], + {'host-block-lists': [blocklist], 'host-blocking-enabled': True, 'host-blocking-whitelist': None}} host_blocker = adblock.HostBlocker()