Skip to content

Fix tests for PySide2 and add PySide2 to CI matrix#484

Merged
kitchoi merged 7 commits into
masterfrom
ci-test-with-pyside2
Dec 18, 2020
Merged

Fix tests for PySide2 and add PySide2 to CI matrix#484
kitchoi merged 7 commits into
masterfrom
ci-test-with-pyside2

Conversation

@kitchoi

@kitchoi kitchoi commented Dec 9, 2020

Copy link
Copy Markdown
Contributor

Fix #382
Fix #495

Edited message:

The segfault observed was caused by an incorrect use of the QWheelEvent API. In Qt4, the QWheelEvent's pos and globalpos takes a QPoint type (see doc). However, since Qt5, QWheelEvent API is changed and the pos and globalpos are QPointF instead (see doc, and there are other API changes). For backward compatibility, PySide2/Qt5's QWheelEvent has an overloaded signature to support both, so that code can migrate. If the signature matches that of Qt4, QPoint is expected for pos and globalpos. If the signature matches that of Qt5, QPointF is expected.

This can be demonstrated with the following script:

from PySide2 import QtCore, QtGui

qt4_args = (
    QtCore.QPoint(0, 0),    # pixelDelta
    QtCore.QPoint(0, 200),  # angleDelta
    200,                    # qt4Delta
    QtCore.Qt.Vertical,     # qt4Orientation
    QtCore.Qt.NoButton,     # buttons
    QtCore.Qt.NoModifier,   # modifiers
    QtCore.Qt.ScrollUpdate, # phase
)

# Qt 5 / Qt 6 args
qt_args = (
    QtCore.QPoint(0, 0),    # pixelDelta
    QtCore.QPoint(0, 200),  # angleDelta
    QtCore.Qt.NoButton,     # buttons
    QtCore.Qt.NoModifier,   # keyboard modifiers
    QtCore.Qt.ScrollUpdate, # phase
    False,                  # inverted
)

# Using Qt4 API
print("Using Qt4 API with QPoint")
event = QtGui.QWheelEvent(
    QtCore.QPoint(0, 0), QtCore.QPoint(0, 0), *qt4_args
)

# Using Qt5 API...
print("Using Qt5 API")
event = QtGui.QWheelEvent(
    QtCore.QPointF(0, 0), QtCore.QPointF(0, 0), *qt_args
)

# Using Qt4 API... this segfault!
print("Using Qt4 API with QPointF, this segfault")
QtGui.QWheelEvent(
    QtCore.QPointF(0, 0), QtCore.QPointF(0, 0), *qt4_args
)

Running this script prints the following:

Using Qt4 API with QPoint
Using Qt5 API
Using Qt4 API with QPointF, this segfault
Segmentation fault: 11

Note that in PySide6/Qt6, the backward compatibility layer for Qt4 is removed, so we will have to update these tests again. But I will defer that to a separate PR.

Original message:

I started with trying PySide2 from EDM. Previously segfaults were observed in Pyface (enthought/pyface#765), and I can also reproduce segfault quite consistently on my OSX machine. This is marked as draft so that I can verify the behaviour on CI.
Then I will try to use PySide2 from pip.

@kitchoi

kitchoi commented Dec 9, 2020

Copy link
Copy Markdown
Contributor Author

Yep, got a segfault, at the same test as I have observed on my machine:

test_vertical_mouse_wheel (enable.tests.qt4.test_mouse_wheel.MouseWheelTestCase) ... �[31m�[1mCommand "python -W default -mcoverage run -m unittest discover enable -v" exited with segmentation fault (-11)�[0m

Travis CI log:
enable-segfault-pyside2.txt

I tried PySide2 from PyPI locally and the segfault happens at the same test.
Will investigate.

@kitchoi

kitchoi commented Dec 18, 2020

Copy link
Copy Markdown
Contributor Author

The tests causing the segfault are now fixed. Ready for review.

@kitchoi kitchoi marked this pull request as ready for review December 18, 2020 13:26
@kitchoi kitchoi changed the title [WIP] Include Pyside2 to CI matrix Include Pyside2 to CI matrix Dec 18, 2020
@kitchoi kitchoi changed the title Include Pyside2 to CI matrix Fix test for PySide2 and add Pyside2 to CI matrix Dec 18, 2020
@kitchoi kitchoi changed the title Fix test for PySide2 and add Pyside2 to CI matrix Fix tests for PySide2 and add Pyside2 to CI matrix Dec 18, 2020
@kitchoi kitchoi changed the title Fix tests for PySide2 and add Pyside2 to CI matrix Fix tests for PySide2 and add PySide2 to CI matrix Dec 18, 2020

@rahulporuri rahulporuri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kitchoi kitchoi merged commit 0678764 into master Dec 18, 2020
@kitchoi kitchoi deleted the ci-test-with-pyside2 branch December 18, 2020 15:53
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.

Segfault in test suite when pyside2 installed Update CI for Qt5

2 participants