Don't create ~/.config/qute_test by mocking out standdarddir.config for
all tests in this module.
This adds config_tmpdir to fixtures.py and moves temp_datadir from
test_adblock to fixtures.py as it will be needed more broadly.
Running test_standarddir would pollute the user's home with
`~/.cache/qute_test`.
The `no_cachedir_tag` fixture was supposed to prevent this, but was not
working because [usefixtures does not work on fixtures]
(https://github.com/pytest-dev/pytest/issues/1014).
This fixes the fixture to actually prevent cachedir creation, but
applies it to tests individually (or by class) rather than with autouse
because the cachedir tests cannot pass if it is working.
Running the tests would create ~/.config/qute_test and
~/.local/share/qute_test on the user's machine. The test_standardir
module needed a bit more mocking to prevent it from cluttering the
user's machine.
Two tests that created the data dir were fixed by passing basedir in
args, and one test that created the config dir was fixed by patching
os.makedirs to a noop.
In the last CherryPy update, cherrypy.wsgiserver got converted to a
single module. While this issue still exists in pylint, we don't get it
here anymore.
We have some things like pos_px stubbed which will fail any test because
of the stub warning - but some tests don't actually need that, it just
happens when e.g. loading something.
So let's not fail tests based on stub warnings, and see how much works
that way.
Per one of the diff comments on #1597:
> I used to use a tuple for constant things, but nowadays I'd actually
> prefer a list as a tuple is something more heterogeneous (i.e. it
> makes sense to have a `(x, y)` point as a tuple, but a list of points
> would be a list).
> At some point I should probably change it to a list everywhere 😉
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").
- The invalid URL will now get encoded when using QUrl.
- The check for a None url_text is somewhat pointless as I don't think
this can ever happen in the real circumstances.
There'll be a refactoring to add a session API to WebTab later anyways,
so no point in fixing this now.
As many tests as possible here should probably also be changed to
end2end ones as there's a lot of mocking going on.
For the Completer unit tests:
Although `change_completed_part` looks like a public method, it was
only used internally. Test the externally-used method
`selection_changed` instead.
Based on code review:
- Use qtbot.waitSignal to test a signal firing
- Use pytest.mark.xfail for an expected test failure
- Ensure there are 2 newlines between module-level functions
Based on code review:
- import modules, not classes
- use methods, not lambdas for the mock command prompt class
- use None rather than Mock for DUMB_SORT
- autouse two fixtures and remove them from test signatures
