search

glue-viz / glue-qt

Other
1 stars 5 forks source link

Improve compatibility with PySide #3

Closed astrofrog closed 11 months ago

astrofrog commented 11 months ago

Work in progress as there are still a number of test failures

astrofrog commented 11 months ago

@dhomeier some of the jobs are failing even though pytest succeeds, e.g. https://github.com/glue-viz/glue-qt/actions/runs/5890374867/job/15975357251?pr=3 - is that related to the QThread destroyed error at the end? (just in case you knew from when you looked at PySide errors before)

dhomeier commented 11 months ago

Certainly looks familiar, but I am also wondering whether the

/home/runner/work/glue-qt/glue-qt/.tox/py311-test-pyside64/lib/python3.11/site-packages/glue_qt/conftest.py:102: UserWarning: There are 5 remaining references to GlueApplication

in the pyside64 tests is related, which I don't recall seeing before. But the pyside515 test reports the same QThread error in https://github.com/glue-viz/glue-qt/actions/runs/5890374867/job/15975356771?pr=3#step:10:311 without that warning. The last glue-core tests with Qt all had actual test failures or segfaults with PySide, so can't find anything related there.

astrofrog commented 11 months ago

@dhomeier - ok no problem, thanks!

dhomeier commented 11 months ago

UserWarning: There are 5 remaining references to GlueApplication

Was wondering if that was related to the 5 xfailed tests, but e.g. the pyqt63 and pyqt64 tests have the same numbers and are even warning about 10 remaining references, so it's probably not related.

astrofrog commented 11 months ago

Ok so I managed to set up a proof of concept of how we can manually extract the number of failed tests and errors - now I just need to figure out how to ignore the exit code from just the pytest command as tox currently seems to escape the || true I tried adding before.

astrofrog commented 11 months ago

@dhomeier - just in case you are interested, see the solution here - when using the skipexitcode mode I added to the tox config, we ignore the pytest exit code and instead purely check whether there are any test failures or errors. Hopefully with this we will be able to get rid of the allowed failures. Currently we might still run into the occasional segfault, but maybe we can see how often it happens. We could adjust the skipexitcode mode to silently succeed if a segfault happened to avoid failing the CI.

astrofrog commented 11 months ago

I will go ahead and merge despite the remaining two failures as it is already an improvement!