Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add note about PySide6 exception capture and fix tests #525

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

nicoddemus
Copy link
Member

PySide6 does its own exception capture during events, re-raising them when control gets back to Python.

This feature causes our own exception capturing mechanism to not work, but it is not needed anyway, because the exception re-raising done by PySide6 contains a better traceback than it is possible for pytest-qt to provide, given it can re-raise at the exact point where control was given back to Python.

Besides noting this in the documentation, also accomodate the tests.

@The-Compiler
Copy link
Member

I don't know much about PySide, and haven't looked into this in detail - but shouldn't this be enabled only for PySide 6.5.2+ and 6.6+ rather than all PySide6 versions? A look at the CI reveals that 6.5.1 passed and 6.5.2 started to fail, and some digging in the Qt for Python Development Notes revealed PYSIDE-2335 which indeed was merged in 6.5.2 and 6.6.

@nicoddemus
Copy link
Member Author

nicoddemus commented Oct 10, 2023

I don't know much about PySide, and haven't looked into this in detail

Oh I should have been clear, I also did not look into detail, this was just the behavior I've observed today after attempting to fix the tests -- you did a more through dig than I did. 😁

but shouldn't this be enabled only for PySide 6.5.2+ and 6.6+ rather than all PySide6 versions?

What do you mean by "enabled"? Note that I did not change any production code, only accommodated the tests. This was not really a change in pytest-qt, just a realization that the behavior has changed in PySide 6, and the tests were updated accordingly. I do not think we should support different pyside6 versions in the test suites, if that's what you mean -- we do not have the bandwidth for this kind of "fine" support, a test suite which works in PySide6 <6.5.2 and PySide6 6.5.2+.

Or do you mean to mention 6.5.2+ in added notes to the documentation? That is fair.

@nicoddemus
Copy link
Member Author

@The-Compiler gentle ping.

@The-Compiler
Copy link
Member

Whoops, sorry for the delay! I have quite a backlog of stuff after ignoring my mails for a while because I moved...

I thought we already tested with different Qt versions, but indeed we don't (it's something I do for qutebrowser). So I agree it's fine to leave this as-is. Would indeed be good to clarify this in the docs though, to avoid confusing people with that statement.

PySide6 does its own exception capture during events, re-raising them when control gets back to Python.

This feature causes our own exception capturing mechanism to not work, but it is not needed anyway, because the exception re-raising done by PySide6 contains a better traceback than it is possible for pytest-qt to provide, given it can re-raise at the exact point where control was given back to Python.

Besides noting this in the documentation, also accomodate the tests.
ReadTheDocs no longer installs rtd-theme by default in its virtual environments, so we
need to explicitly install it.
@nicoddemus
Copy link
Member Author

Thanks!

@nicoddemus nicoddemus merged commit 62c36c1 into pytest-dev:master Oct 19, 2023
62 checks passed
@nicoddemus nicoddemus deleted the pyside6-capture branch October 19, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants