rnhmjoj/qutebrowser
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

tests: fix tests for downloads bdd test

Browse Source 
The test was failing because of two reasons:

First, the old code had filename questions in DownloadManager.get and
DownloadManager.fetch which were almost identical, thus the part in
DownloadManager.get was removed in an earlier commit. All filename
asking is now done by DownloadManager.fetch. The good part is code
deduplication, the bad part is slightly modified behavior: The new code
doesn't wait for a filename to start the download, instead it tries to
fill the buffer immediately. This made the test fail because qute:// has
no registered handler, so in order for the test to pass now, the "no
crash" part is not enough, we also need to expect the "No handler"
error.

Secondly, and a rather rare (race) condition was the handling of errors
in the DownloadItem. If an error occured after the registration of
self.on_reply_error as error handler and before the check
    reply.error() != QNetworkReply.NoError
at the end of the function, the error signal would be emitted twice:
Once by _die() (called by on_reply_error), and once by the init_reply
function directly (in the last if block). This lead to duplicated error
messages. This is also explained in a comment in the file (with small
"stack traces").
This commit is contained in:
Daniel Schadt 2016-07-07 00:08:13 +02:00
parent 7608805c9a
commit 8795f89c88
2 changed files with 19 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
qutebrowser/browser/webkit/downloads.py
View File

@ -281,6 +281,7 @@ class DownloadItem(QObject):
_read_timer: A Timer which reads the QNetworkReply into self._buffer
periodically.
_win_id: The window ID the DownloadItem runs in.
_dead: Whether the Download has _die()'d.
Signals:
data_changed: The downloads metadata changed.
@ -329,6 +330,7 @@ class DownloadItem(QObject):
self.init_reply(reply)
self._win_id = win_id
self.raw_headers = {}
self._dead = False
def __repr__(self):
return utils.get_repr(self, basename=self.basename)
@ -396,6 +398,21 @@ class DownloadItem(QObject):
def _die(self, msg):
"""Abort the download and emit an error."""
assert not self.successful
# Prevent actions if calling _die() twice. This might happen if the
# error handler correctly connects, and the error occurs in init_reply
# between reply.error.connect and the reply.error() check. In this
# case, the connected error handlers will be called twice, once via the
# direct error.emit() and once here in _die(). The stacks look like
# this then:
# <networkmanager error.emit> -> on_reply_error -> _die ->
# self.error.emit()
# and
# [init_reply -> <single shot timer> ->] <lambda in init_reply> ->
# self.error.emit()
# which may lead to duplicate error messages (and failing tests)
if self._dead:
return
self._dead = True
self._read_timer.stop()
self.reply.downloadProgress.disconnect()
self.reply.finished.disconnect()
@ -442,7 +459,7 @@ class DownloadItem(QObject):
# Here no signals are connected to the DownloadItem yet, so we use a
# singleShot QTimer to emit them after they are connected.
if reply.error() != QNetworkReply.NoError:
QTimer.singleShot(0, lambda: self.error.emit(reply.errorString()))
QTimer.singleShot(0, lambda: self._die(reply.errorString()))
def get_status_color(self, position):
"""Choose an appropriate color for presenting the download's status.

3
tests/end2end/features/downloads.feature
View File

@ -31,8 +31,7 @@ Feature: Downloading things from a website.
And I run :hint links download
And I run :follow-hint a
And I wait for "Asking question <qutebrowser.utils.usertypes.Question default='qutebrowser-download' mode=<PromptMode.download: 5> text='Save file to:'>, *" in the log
And I run :leave-mode
Then no crash should happen
Then the error "Download error: No handler found for qute://!" should be shown
Scenario: Downloading a data: link (issue 1214)
When I set completion -> download-path-suggestion to filename