Closed zoshua closed 4 months ago
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.
:white_check_mark: zoshua
:white_check_mark: masqu3rad3
:x: joshochoa
Thanks for this. We need tests for this to happen, something to run the code and make sure it works alongside all the other tests but for Qt 6.
Can you have a look at https://github.com/mottosso/Qt.py/blob/master/.travis.yml and see how we could get Qt.py running with access to PySide6?
We'd also need mention in the README that PySide6 is supported, along with PyQt6.
@mottosso Any chance we can call in for an assist on that part. It would definitely be my first time wading into those waters and would definitely appreciate guidance on how to proceed.
Calling in all assists!
Does anyone feel up to the task of updating our test suite for Qt 6? We're currently on Travis-CI, because in the past it used to be free.. So part of the challenge would be converting it into GitHub Actions. The next challenge is updating our Docker image for the test environment, which is currently very very complicated hahaha. 😅 So a simplified testing environment would be fine, something like a native Ubuntu that GitHub Actions provides natively.
Why do all this? Because if you look at virtually every PR to this repository, all of them will have something non-obvious that needs addressing. In particular adding members to Qt.py that shouldn't be there and would break things for someone. It's hard for a single person to test every Python with every Qt binding, especially when introducing 2 more like this PR does. So, it's a challenge, but the hard reality is that it's better to have a limited but working Qt.py than a feature-complete but buggy Qt.py
https://github.com/mottosso/Qt.py/blob/master/Qt.py#L851 needs to be adapted to catch PySide6 too..
@mottosso not sure what went all into your docker image, but would a simple ubuntu with pip install PySide6 and then executing unitests suffice ? and GitHub Actions then have the nice matrix to do this under windows too and multiple python versions.. but I guess I would make one yaml per binding, PySide2 and PySide6 being the "simple" ones at least for py3 as they can be pulled via pip.
but I am far from being an expert around GitHub actions... I made a setup once and I am glad that it still works...
https://github.com/nebukadhezer/Qt.py/pull/1/files
Here I made a small sample PR in my fork.. I placed the tests to run upon merge in master. I only added python38 and the tests are now crippled to PySide6 only... (and they do fail, locally on a Mac 4 of 30 fail, now on ubuntu 21 of 30 fail... have not looked into, not the point, more to convey the idea) let me know if this in your interest then I can continue...
Idea would be to have a workflows per binding and call the tests per binding...
not sure what went all into your docker image, but would a simple ubuntu with pip install PySide6 and then executing unitests suffice ?
It looks (and is) complicated because we're testing against the version of Python and Qt officially part of the VFX Platform, and in that platform there is no Qt 6 yet.
Can we test against vanilla Python and vanilla Qt instead..? Probably. But the challenge is finding some way of testing against all versions covered by Qt.py. Because it still needs to support Qt 4 and Qt 5, along with Python 2 and 3.
So your approach of adding a GitHub workflow is good, but:
How about adding one of these workflows per version of Qt? That way, you can install an old version of Python and Ubuntu etc. in the workflow, and isolate that environment there. Rather than trying to do what we did before, to run every test for every version in the same workflow.
hey, sounds good was also what I meant with "one yaml(workflow) per binding"... I am adding them now gradually and with one python variant as a start. But when it comes to unit tests failing I might need some help... and currently I run pytest test.py I just so the run_tests.py... Should I use nose with the same settings ? nosetests --verbose --exe
And in Qt6 there is a module QtX11Extras gone. I can adapt the unittest to check modules too, but I am not sure within Qt.py how to specify if a module is missing for a specific binding... or we exclude the module in the unitest for Qt6 bindings.
Are you still on gitter ?
The call made at the moment is this one here.
https://github.com/mottosso/Qt.py/blob/master/Dockerfile.vfxplatform2018#L807
cp -r /Qt.py /workdir && chmod +x entrypoint.sh && ./entrypoint.sh
Which boils down to..
./entrypoint.sh
Which is here.
python2.7 -u run_tests.py
In addition to python2.7 build_caveats.py
and testing the example files too. But you can start with just the first one. And yes, that's tests based on nose, so stick with that to avoid having to rewrite any of the tests.
And in Qt6 there is a module QtX11Extras gone. I can adapt the unittest to check modules too, but I am not sure within Qt.py how to specify if a module is missing for a specific binding... or we exclude the module in the unitest for Qt6 bindings.
Anything that doesn't exist in all bindings should not be part of Qt.py, so there is no need to test for those modules. They don't exist.
Are you still on gitter ?
Yes, but it would be best to keep the conversation here since it's a major update.
QAction is missing
Linking this conversation about how exactly to incorporate PySide6 - and Qt 6 in general - with Qt.py
@mottosso wrote:
Calling in all assists!
(...) We're currently on Travis-CI, because in the past it used to be free.. So part of the challenge would be converting it into GitHub Actions. (...)
The Travis conversion has since been merged (thanks to @martin-chatterjee!), so it should be more feasible to write tests for this one and get the show on the road. 🥳
@zoshua A first step is to merge with latest master, such that your PR also runs in this new test environment.
@mottosso Merged with latest. Tested locally in fresh PySide6 env, Maya 2024, 2023.
Unsure why tests aren't running, so I'll merge this to a temp branch to kick things off.
There we go, have a look at these broken tests, and submit a new PR please. The new PR should trigger these tests for each commit, making it a bit easier to test. You can also run the tests locally, to speed up iteration time further.
https://github.com/nebukadhezer/Qt.py/pull/1/files
there is still the effort I once put in to run the tests in github actions for pyside6... maybe that is useful to look at.
Addressed Need for PySide6 and shiboken6 support in Blender 3.3.1