Greasemonkey: handle downloads that complete fast
When `@require`ing local files (with the `file://` scheme) the greasemonkey manager was not catching the DownloadItem.finished signal because it was being emitted before it had managed to connect. I didn't see this happening while testing with files that should have been in cache but I wouldn't be surprised. I had to change the download mock to be able to give it the appearance of asynchronicity. Now when using it one must set download.successful appropriately before firing download.finished. I also added a list of downloads to the stub so a test could enumerate them in case the unit-under-test didn't have a reference to them.
This commit is contained in:
parent
87a0c2a7a7
commit
7dab8335e2
@ -287,9 +287,12 @@ class GreasemonkeyManager(QObject):
|
|||||||
auto_remove=True)
|
auto_remove=True)
|
||||||
download.requested_url = url
|
download.requested_url = url
|
||||||
self._in_progress_dls.append(download)
|
self._in_progress_dls.append(download)
|
||||||
download.finished.connect(
|
if download.successful:
|
||||||
functools.partial(self._on_required_download_finished,
|
self._on_required_download_finished(script, download)
|
||||||
script, download))
|
else:
|
||||||
|
download.finished.connect(
|
||||||
|
functools.partial(self._on_required_download_finished,
|
||||||
|
script, download))
|
||||||
|
|
||||||
def scripts_for(self, url):
|
def scripts_for(self, url):
|
||||||
"""Fetch scripts that are registered to run for url.
|
"""Fetch scripts that are registered to run for url.
|
||||||
|
@ -530,5 +530,5 @@ def download_stub(win_registry, tmpdir, stubs):
|
|||||||
"""Register a FakeDownloadManager."""
|
"""Register a FakeDownloadManager."""
|
||||||
stub = stubs.FakeDownloadManager(tmpdir)
|
stub = stubs.FakeDownloadManager(tmpdir)
|
||||||
objreg.register('qtnetwork-download-manager', stub)
|
objreg.register('qtnetwork-download-manager', stub)
|
||||||
yield
|
yield stub
|
||||||
objreg.delete('qtnetwork-download-manager')
|
objreg.delete('qtnetwork-download-manager')
|
||||||
|
@ -572,7 +572,7 @@ class FakeDownloadItem(QObject):
|
|||||||
super().__init__(parent)
|
super().__init__(parent)
|
||||||
self.fileobj = fileobj
|
self.fileobj = fileobj
|
||||||
self.name = name
|
self.name = name
|
||||||
self.successful = True
|
self.successful = False
|
||||||
|
|
||||||
|
|
||||||
class FakeDownloadManager:
|
class FakeDownloadManager:
|
||||||
@ -581,6 +581,7 @@ class FakeDownloadManager:
|
|||||||
|
|
||||||
def __init__(self, tmpdir):
|
def __init__(self, tmpdir):
|
||||||
self._tmpdir = tmpdir
|
self._tmpdir = tmpdir
|
||||||
|
self.downloads = []
|
||||||
|
|
||||||
@contextlib.contextmanager
|
@contextlib.contextmanager
|
||||||
def _open_fileobj(self, target):
|
def _open_fileobj(self, target):
|
||||||
@ -603,4 +604,5 @@ class FakeDownloadManager:
|
|||||||
download_item = FakeDownloadItem(target.fileobj, name=url.path())
|
download_item = FakeDownloadItem(target.fileobj, name=url.path())
|
||||||
with (self._tmpdir / url.path()).open('rb') as fake_url_file:
|
with (self._tmpdir / url.path()).open('rb') as fake_url_file:
|
||||||
shutil.copyfileobj(fake_url_file, download_item.fileobj)
|
shutil.copyfileobj(fake_url_file, download_item.fileobj)
|
||||||
|
self.downloads.append(download_item)
|
||||||
return download_item
|
return download_item
|
||||||
|
@ -206,6 +206,7 @@ def test_disabled_blocking_update(basedir, config_stub, download_stub,
|
|||||||
while host_blocker._in_progress:
|
while host_blocker._in_progress:
|
||||||
current_download = host_blocker._in_progress[0]
|
current_download = host_blocker._in_progress[0]
|
||||||
with caplog.at_level(logging.ERROR):
|
with caplog.at_level(logging.ERROR):
|
||||||
|
current_download.successful = True
|
||||||
current_download.finished.emit()
|
current_download.finished.emit()
|
||||||
host_blocker.read_hosts()
|
host_blocker.read_hosts()
|
||||||
for str_url in URLS_TO_CHECK:
|
for str_url in URLS_TO_CHECK:
|
||||||
@ -221,6 +222,8 @@ def test_no_blocklist_update(config_stub, download_stub,
|
|||||||
host_blocker = adblock.HostBlocker()
|
host_blocker = adblock.HostBlocker()
|
||||||
host_blocker.adblock_update()
|
host_blocker.adblock_update()
|
||||||
host_blocker.read_hosts()
|
host_blocker.read_hosts()
|
||||||
|
for dl in download_stub.downloads:
|
||||||
|
dl.successful = True
|
||||||
for str_url in URLS_TO_CHECK:
|
for str_url in URLS_TO_CHECK:
|
||||||
assert not host_blocker.is_blocked(QUrl(str_url))
|
assert not host_blocker.is_blocked(QUrl(str_url))
|
||||||
|
|
||||||
@ -238,6 +241,7 @@ def test_successful_update(config_stub, basedir, download_stub,
|
|||||||
while host_blocker._in_progress:
|
while host_blocker._in_progress:
|
||||||
current_download = host_blocker._in_progress[0]
|
current_download = host_blocker._in_progress[0]
|
||||||
with caplog.at_level(logging.ERROR):
|
with caplog.at_level(logging.ERROR):
|
||||||
|
current_download.successful = True
|
||||||
current_download.finished.emit()
|
current_download.finished.emit()
|
||||||
host_blocker.read_hosts()
|
host_blocker.read_hosts()
|
||||||
assert_urls(host_blocker, whitelisted=[])
|
assert_urls(host_blocker, whitelisted=[])
|
||||||
@ -265,6 +269,8 @@ def test_failed_dl_update(config_stub, basedir, download_stub,
|
|||||||
# if current download is the file we want to fail, make it fail
|
# if current download is the file we want to fail, make it fail
|
||||||
if current_download.name == dl_fail_blocklist.path():
|
if current_download.name == dl_fail_blocklist.path():
|
||||||
current_download.successful = False
|
current_download.successful = False
|
||||||
|
else:
|
||||||
|
current_download.successful = True
|
||||||
with caplog.at_level(logging.ERROR):
|
with caplog.at_level(logging.ERROR):
|
||||||
current_download.finished.emit()
|
current_download.finished.emit()
|
||||||
host_blocker.read_hosts()
|
host_blocker.read_hosts()
|
||||||
@ -294,16 +300,18 @@ def test_invalid_utf8(config_stub, download_stub, tmpdir, data_tmpdir,
|
|||||||
|
|
||||||
host_blocker = adblock.HostBlocker()
|
host_blocker = adblock.HostBlocker()
|
||||||
host_blocker.adblock_update()
|
host_blocker.adblock_update()
|
||||||
finished_signal = host_blocker._in_progress[0].finished
|
current_download = host_blocker._in_progress[0]
|
||||||
|
|
||||||
if location == 'content':
|
if location == 'content':
|
||||||
with caplog.at_level(logging.ERROR):
|
with caplog.at_level(logging.ERROR):
|
||||||
finished_signal.emit()
|
current_download.successful = True
|
||||||
|
current_download.finished.emit()
|
||||||
expected = (r"Failed to decode: "
|
expected = (r"Failed to decode: "
|
||||||
r"b'https://www.example.org/\xa0localhost")
|
r"b'https://www.example.org/\xa0localhost")
|
||||||
assert caplog.records[-2].message.startswith(expected)
|
assert caplog.records[-2].message.startswith(expected)
|
||||||
else:
|
else:
|
||||||
finished_signal.emit()
|
current_download.successful = True
|
||||||
|
current_download.finished.emit()
|
||||||
|
|
||||||
host_blocker.read_hosts()
|
host_blocker.read_hosts()
|
||||||
assert_urls(host_blocker, whitelisted=[])
|
assert_urls(host_blocker, whitelisted=[])
|
||||||
|
Loading…
Reference in New Issue
Block a user