| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
useful for tools showing coverage info in e.g. your editor
|
|
|
|
|
|
|
|
|
|
|
|
|
|
This seems to help with severe flakiness around clicking elements / JS
execution on Qt 6.4+.
See https://bugreports.qt.io/browse/QTBUG-112017
and #5390
|
|
|
|
Update dependencies
|
|
Contrary to what I thought at the time when initially writing this,
typing.AnyStr isn't just an alias for IO[str] | IO[bytes], but is actually a
TypeVar. As per the Python docs, it should be used when there are *multiple*
places where the types need to match:
def concat(a: AnyStr, b: AnyStr) -> AnyStr:
return a + b
What we do instead is somewhat akin to "def fun() -> T:", which mypy already
comments on:
error: A function returning TypeVar should receive at least one argument
containing the same TypeVar [type-var]
def t() -> T:
Not quite sure why it doesn't in this case, or why it now raises an additional
error (possibly the new inferrence code or something?). Either way, with this
commit the annotations are now more correctly using Union[IO[str], IO[bytes]],
including typing.Literal overloads so that mypy actually knows what specific
type will be returned by a call.
|
|
|
|
PyQt 6.6 has been out for a while. Git uses on arch are already using
it. Likely our next pyinstaller release will be using it. This change
adds it to our test matrix, beyond the arch docker tests.
* Removing -dev tag from python 3.12 job
* Update ubuntu python 3.11 and 3.12 tests to use PyQt6.6
* Update macOS and windows tests to use PyQt6.6
* Allow running the nightly CI job on any branch, to get a pyinstaller
build binary from your own branch
Closes: #7989
|
|
We think that at some point pytest added color codes to the summary
output which broke these matcher regex. It looks like there is an issue
about stripping color codes here: https://github.com/actions/runner/issues/2341
I've added wildcard (or no-space) elements instead of trying to match
the color codes exactly (eg `\033\[31m` etc) because 1) that's not very
readable 2) I was having trouble getting that to work with egrep.
The goal is to match the target strings but not to match then later in
the line as part of a test log or whatever. For the '= short test
summary =' that should be pretty unique. For the ERROR|FAILED I reckon
that could be a bit more common. I'm just matching with any mount of
non-space characters because I reckon a space will crop up pretty early
in any line where ERROR isn't the first work.
I think this new matching will only apply to new or rebased PRs after it
is landed.
ref: https://github.com/qutebrowser/qutebrowser/issues/5390#issuecomment-1817503702
|
|
In the linux branch when it was doing:
header, *lines, empty, result = text.split("\n")
assert not empty
It was complaining that "empty" was "}", because the windows sandbox
page has JSON at the bottom now. The whole things looks to have changed
completely. I'm actually surprised it was working before, why would it
have been saying seccomp was enabled on windows?
Anyway, I did the debug-dump-text --plain that quteproc is doing in a VM
and tested this with sandboxing off an on. No idea how stable that will
be!
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
|
|
The nightly jobs have a `workflow_dispatch` action, which means you can
kick the job off on any branch. But the build steps has the branch to
build on hardcoded. I would like to be able to build windows and mac
builds without having a local build environment setup.
The docs for the checkout action says it default to the main branch,
so the scheduled actions should keep working fine. But now we'll be able
to create builds off of other branches too.
docs: https://github.com/actions/checkout#usage
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
|
|
It looks like our last release builds were done with python 3.11 and
PyQt 6.5.3. I'm expecting that since PyQt6.6 is out now our next release
will be on 6.6. So lets update the CI to match.
Questions:
* what about python12? I don't think there is a benefit to updating to
that, so lets leave it.
* what about pyqt6.5? Do we care about testing that? Maybe for homebrew
users? We aren't providing new builds with an old Qt right?
last release builds: https://github.com/qutebrowser/qutebrowser/actions/runs/6578864884
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
|
|
I'm not sure if we need a py3.11 pyqt6.5 variant or a py3.10 pyqt6.6
one? Those might well be combinations that people have (debian has 3.11
and 6.5 at the moment) but how much coverage do we need?
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
|
|
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
|
|
Update dependencies
|
|
|
|
qutebrowser/dependabot/github_actions/actions/github-script-7
build(deps): bump actions/github-script from 6 to 7
|
|
Bumps [actions/github-script](https://github.com/actions/github-script) from 6 to 7.
- [Release notes](https://github.com/actions/github-script/releases)
- [Commits](https://github.com/actions/github-script/compare/v6...v7)
---
updated-dependencies:
- dependency-name: actions/github-script
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
|
|
I believe we are being afflicted by this issue: https://github.com/python/mypy/issues/16451
Although I'm not 100% sure because there is a lot going on in this
function and I haven't managed to grok it.
The mypy 1.7 release [notes][1.7] say you can disable the new type
inference by running `tox -e mypy-pyqt6 -- --old-type-inference` and
indeed mypy passes with that. So either our type hints are incorrect or
we are hitting a bug. Considering the inferred type hint has a `Never`
in it I'm leading toward it being a bug. So I'll bump the mypy version
down and hopefully next week the issue will be resolved.
The mypy output before this commit was:
mypy-pyqt6: commands[0]> .tox/mypy-pyqt6/bin/python -m mypy --always-true=USE_PYQT6 --always-false=USE_PYQT5 --always-false=USE_PYSIDE6 --always-false=IS_QT5 --always-true=IS_QT6 --always-true=IS_PYQT --always-false=IS_PYSIDE qutebrowser
qutebrowser/utils/qtutils.py:239: error: Argument 1 to "contextmanager" has incompatible type
"Callable[[str, bool, str], Iterator[IO[AnyStr]]]"; expected "Callable[[str, bool, str], Iterator[IO[Never]]]" [arg-type]
@contextlib.contextmanager
^
qutebrowser/misc/lineparser.py: note: In member "save" of class "LineParser":
qutebrowser/misc/lineparser.py:168: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._configfile, self._binary) as f:
^
qutebrowser/misc/lineparser.py: note: In member "save" of class "LimitLineParser":
qutebrowser/misc/lineparser.py:226: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._configfile, self._binary) as f:
^
qutebrowser/config/configfiles.py: note: In member "_save" of class "YamlConfig":
qutebrowser/config/configfiles.py:292: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._filename) as f:
^
qutebrowser/misc/sessions.py: note: In member "save" of class "SessionManager":
qutebrowser/misc/sessions.py:343: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(path) as f:
[1.7]: https://mypy-lang.blogspot.com/2023/11/mypy-17-released.html
|
|
Didn't manage to merge the first one in time.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
What is that big chain of !pyqt- etc mean? idk
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be None, we've been relying on them
being set for long enough.
`qWebEngineVersion()` and `qWebEngineChromiumVersion()` now are typed as
returning `Optional[str]`. In `from_api()` we can handle the
`chromium_version` being null, so pass that through, but we are depending on
the `qtwe_version` being set, so add an assert there.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be none though so I'm adding
some low effort null checking in here.
For the changes in `_SettingsWrapper` I'm not actually sure this is the
best way to be resolving this. mypy was complaining that `settings()` might be
None. How does asserting on the profile help? idk but that seems to make it
happy. Even more weirdly, none of these getters I changed seemed to be used
anywhere apart from tests. Except for getAttribute, that's used in webkit code.
Also the whole thing is checking the global default profile, generally
settings should be checked on the profile on the tab. So I think this might
just be some old code? idk, I just want to make mypy happy.
For the `init_user_agent()` change that was complaining about the profile
maybe being None.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
The type of the two list arguments of chooseFiles() have changed from
`Iterable[str]` to `Iterable[Optional[str]]`.
I'm not sure it makes much sense to have individual list elements as
None. That seems like a pretty unlikely case. So we could just put an
ignore comment somewhere. I've added another copy of the lists, with
longer names, to strip any hypothetical Nones out. This allows us to be
backwards compatible with the old type hints (and the current Qt5 ones),
since that doesn't have the Optional part in the signature.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be none though so I'm sprinkling
some low effort null checking in here.
Another alternative is to move the inspector classes to be based off, and to
work with, our overridden child classes of the view and page. But that's a
whole other piece of work, we might have to deal with signals and such meant
for web views that we don't want with the inspector. (On the other hand it
might actually be good. The inspector sometimes is surprising in how it acts
differently from the main views.)
There's a bit later on where I changed a local variable name from
`inspector_page` to `new_page`. The `inspector_page` variable is used later
and the type checker was complaining.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be none though. So in a low effort
way of keeping the types we have to deal with in the calling code to
what they were before I'm overriding these getters that just do `assert thing
not None` to tell mypy not to worry about that case.
QWebEngineView.page()
The docs don't say anything but the source says it should always return
something: https://github.com/qt/qtwebengine/blob/6.6.0/src/webenginewidgets/api/qwebengineview.cpp#L1001
QWebEngineView.history()
calls QWebEnginePage.history()
where it is always set in the constructor: https://github.com/qt/qtwebengine/blob/6.6.0/src/core/api/qwebenginepage.cpp#L90
QWebEngineView.settings()
calls QWebEnginePage.settings()
where it is always set in the constructor: https://github.com/qt/qtwebengine/blob/6.6.0/src/core/api/qwebenginepage.cpp#L90
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
Changing the type to be our overridden implementation of the view means
we can push more common code down into the view class and make use of
overridden methods more consistently. In this case I'm adding some type
checking to some of the getters on the view.
`_set_widget()` should be being called from the concrete child tab
implementations. So the widget should always be our overridden version of
the Qt tab implementations.
Also change `_set_widget()` to use the common typevar.
Also change the _widget type in WebEngineTab to match what it is
actually being set to and to match all the _widgets in the other
concrete classes in that file.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
|
|
|
|
|
|
|
|
|
|
Seems mypy and I agree on something. It thinks that pathlib is abusing
the division operation and making python a less intuitive
language. Not sure why it is complaining though really, doing `Path /
"one" / "two"` seems to work fine in practice, and its on the pathlib
documentation page.
I was seeing this error locally and on CI
qutebrowser/misc/pakjoy.py:155: error: Unsupported left operand type for / ("str") [operator]
resources_dir /= "lib" / "QtWebEngineCore.framework" / "Resources"
|
|
|
|
I have no idea in what case these errors would crop up. But vulture said
the function was unused, not it's used.
|
|
On CI I'm seeing:
No such file or directory: '/Users/runner/work/qutebrowser/qutebrowser/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/resources'
Downloading the PyQt6_WebEngine_Qt6 wheel for mac from PyPI I can see the pak
file is at:
./PyQt6/Qt6/lib/QtWebEngineCore.framework/Resources/qtwebengine_resources.pak
And on a qutebrowser install made by pyinstaller it is at (symlinks in curly
braces):
/Applications/qutebrowser.app/Contents/{Resources,Frameworks}/PyQt6/Qt6/lib/QtWebEngineCore.framework/{Resources,Versions/A/Resources}/qtwebengine_resources.pak
And the Qt data path for reference is:
/Applications/qutebrowser.app/Contents/Frameworks/PyQt6/Qt6
So it looks like essentially we need to add a "lib/QtWebEngineCore.framework"
in there and capitalise "Resources".
A bit annoying to have the special case and hardocde paths like this. But it
should be pretty stable? I had a look at importlib_resources to try to see if
it could fine this stuff for us but I don't think it can.
I've checked with the pyinstaller windows builds and with the windows wheel
from PyPI and neither seem to need any special handling.
|
|
There is one test which does does the whole run through with the real
resources file from the Qt install, patches is to the cache dir, reads
it back and checks the json. All the other tests either use constructed
pak files or stop on earlier error cases.
For testing the actual parsing of the pak files I threw together a quick
factor method to make them.
I went back and forth over the parse code and looked for more error
cases, but it looks pretty solid to me
|
|
The "doesn't exist at expected location" error message is a bit
misleading, it actually prints the location in the directory we copied
stuff to instead of the source one. Oh well, it's good enough.
|
|
Fixes lint: shadows variables, docstrings, unused attribute
Move resource dir copying to its own method:
* will probably help with unit tests
* allows for cleaner overriding of what file to patch (instead of a big
nested block)
Adds a sneaky `assert` statement in there because the pak file should
definitely exist at that location. Although it not existing shouldn't
really be fatal...
Maintains the thing in `__main__`, although I'm not sure if we need to
keep that?
|
