This is not all of the tests that are being flaky. Just the few I looked
at the other day. Hopefully some of them keep failing in this much
smaller set than our usual test suite, otherwise I'll go add some more.
Not sure what I'm going to add in next, probably some code to probe the
state of the page. Run a simple event handler we know works in
isolation or something. Make sure things are still registered. Turn on
chrome debug logs?
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
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
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
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
Florian Bruhin
toofar
dependabot[bot]
qutebrowser bot