The previous fix in 3dc212a815 was insufficient,
as the inner `getattr(extract_result, "registered_domain")` was always evaluated
first (thus triggering the deprecation warning again).
We also cannot do:
getattr(extract_result, "top_domain_under_public_suffix", None) or extract_result.registered_domain
as `""` is a valid value for it.
I just searched for qt5 and deleted stuff. EZ.
Will leave on a branch for a bit and see if I feel like testing this at
all, otherwise maybe leave this stuff in here and make it not called.
Not 100% sure that we need to remove all this stuff when we just want
the CI to go green. But tbh if we don't need to make Qt5 releases then
we don't need it. Better to be bold and pull it out than have to work
around it in the future. And we can always revert the commit.
See 433074c681, this is the same cause. An older version of a
package being included in requirements files because setuptools injects
its vendored packages into sys.path and we use pip freeze to build lock
files. Then when you install two requirements files at the same time they
end up having conflicting versions. This at least means we include the
latest version, which will do until we move to a method of generating
lock files that just works off of the raw requirements file.
Currently the dependency update job is failing[1] because one of the
tests installs multiple requirements files before running the tests and
it claims they have conflicting versions of `importlib_resources` (6.4.0
vs 6.4.4). 6.4.0 is in the pinned files and there is a 6.4.4 available.
Looking though the logs the first time I see importlib_resources==6.4.0
is when printing the requirements for the `test` requirements file.
But it's not mentioned at all when installing that file. Which makes me
think it found it's way into the virtualenv by some other means.
Looking at git blame for the test requirements lock file, it looks like
importlib_resources was introduced in
https://github.com/qutebrowser/qutebrowser/pull/8269 and indeed I can
see version 6.4.0 in setuptools vendored folder[2].
So it looks like this is another issue caused by setuptools adding their
vendored packages into sys.path.
Options I can see for resolving this:
a. add importlib_resources as a dependency in requirements.txt-raw so
that we always pull down the newest one, even though we don't need it
b. add an @ignore line for importlib_resources
* I think in the unlikely event we end up needing it then it being
ignored might be hard to spot
c. drop python 3.8 support
d. switch to a requirements compilation method that doesn't use `pip
freeze`
I've chosen (a) here because I think it's less surprising than (b), less
work than (c) and I already have a PR up for (d). And it's only pulled
down for 3.8 anyhow, so we'll drop this workaround when we drop that.
[1]: https://github.com/qutebrowser/qutebrowser/actions/runs/10660624684/job/29544897516
[2]: https://github.com/pypa/setuptools/tree/main/setuptools/_vendor
This commit takes a screenshot of the active browser window when an
end2end test fails. When running on CI a zip file of screenshots will be
attached to the run summary as an artifact. When run locally screenshots
will be left in /$TMPDIR/pytest-screenshots/.
The screenshot is of the Xvfb screen that the tests are running under.
If there are multiple windows open it will likely only show the active
window because a) we aren't running with a window manager b) the Xvfb
display is, by default, the same size as the browser window.
I'm not sure if xvfb is used on the Window runs in CI. We could fall
back to trying to take screenshots if not running under xvfb but I'm a
bit wary of an automatic feature that takes screenshots of people's
desktops when running locally. Even if they just to to /tmp/ it might be
surprising. We can change it later if it turns out we need to try to
take screenshots in more cases.
I'm using pillow ImageGrab, the same as pyvirtualdisplay.smartdisplay. I'm
getting the display number from the pytest-xvfb plugin and formatting it
appropriately (pyvirtualdisplay has an already formatted one which is used by
the smartdisplay, but that's not accessible).
Pillow is now a requirement for running the tests. I thought about making
it gracefully not required but I'm not sure how to inform the user with
a warning from pytest, or if they would even want one. Maybe we could
add a config thing to allow not taking screenshots?
I had to bump the colordepth config for pytest-xvfb otherwise pillow
complained that the default 16bit color depth wasn't supported as it
only supports 24bit, see https://github.com/python-pillow/Pillow/blob/1138ea5370cbda5eb328ec949
8c314d376c81265/src/display.c#L898
I'm saving screenshots to a temp dir because I don't want to put them in
my workdir when running locally. I want to clear the directory for each
run so that you don't get confused by looking at old images. I'm not
100% sure about the lifecycle of the process classes though. Eg if we
have two processes they might both try to create the output directory.
I'm using pytest.session.stash to save the directory so perhaps the
lifecycle of the stash will handle that? Not sure.
Ideally the images would be uploaded somewhere where we could click
through and open them in the browser without having to download a zip
file, but I'm not sure how to achieve that.
It would be nice to print in the test logs that a screenshot was saved
and where to. Just so you could copy paste the filename instead of
having to match the sanitized filename against failing test names. But I
don't know how to log stuff from this stage in the pytest lifecycle.
TODO:
* I'm not sure that the screenshot captures the whole browser window?
Maybe the browser windows is bigger than the X11 display?
Closes: #7625
Easier fix instead of 6c4be8ef03.
Seems to get picked up just fine, and shouldn't hurt when it's not needed, as we
don't use --pre. Thus, no development releases should be installed.
6.7 is released now, some distros are already shipping it!
This commit:
1. adds a new 6.7 requirements file (the plain 6 one has already been
updated by the bot)
2. adds a new tox env referring to the new requirements file
3. updates the mac and windows installer jobs to run with pyqt67 with the
assumption we'll be including that in our next release
4. adds two new CI environments for 6.7, one each for python 3.11 and 3.12
(3.12 only came out like 6 months ago)
5. updates a couple of references to the py37 tox env that looked like they
were missed, 3.7 support was dropped in 93c7fdd
6. updates various ubuntu containers to the latest LTS instead of the previous
related one - this is quite unrelated to this change but I thought I would
give it a go, no need to use the old one unless we are specifically testing
on it?
- linters - they use tox but probably use system libraries
- these all run in nested containers anyway, should be fully isolated
- codeql - eh, uses a third party action, check the docs if it fails
- irc - as above
qutebrowser bot
Florian Bruhin
toofar
dezeroku
user202729
Jun Chen
Willow Barraco
bitraid
Tobias Naumann
Evan Chen