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

Flaky test: test_qt_image_qapplication.py::test_sanity #6875

Closed
hugovk opened this issue Jan 9, 2023 · 5 comments
Closed

Flaky test: test_qt_image_qapplication.py::test_sanity #6875

hugovk opened this issue Jan 9, 2023 · 5 comments
Labels

Comments

@hugovk
Copy link
Member

hugovk commented Jan 9, 2023

test_sanity in test_qt_image_qapplication.py is a flaky test that intermittently fails. It often passes when restarted.

It would be good to fix this: fix/rewrite the test, or the code if the problem is there, or maybe even remove it. There are also pytest plugins to re-run flaky tests a number of times.

The failure

Tests/test_qt_image_qapplication.py::test_sanity FAILED                  [ 98%]

================================== FAILURES ===================================
_________________________________ test_sanity _________________________________

tmp_path = WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_sanity7')

    @pytest.mark.skipif(not ImageQt.qt_is_installed, reason="Qt bindings are not installed")
    def test_sanity(tmp_path):
        # Segfault test
        app = QApplication([])
        ex = Example()
        assert app  # Silence warning
        assert ex  # Silence warning
    
        for mode in ("1", "RGB", "RGBA", "L", "P"):
            # to QPixmap
            im = hopper(mode)
            data = ImageQt.toqpixmap(im)
    
            assert isinstance(data, QPixmap)
            assert not data.isNull()
    
            # Test saving the file
            tempfile = str(tmp_path / f"temp_{mode}.png")
            data.save(tempfile)
    
            # Render the image
            qimage = ImageQt.ImageQt(im)
            data = QPixmap.fromImage(qimage)
            qt_format = QImage.Format if ImageQt.qt_version == "6" else QImage
            qimage = QImage(128, 128, qt_format.Format_ARGB32)
            painter = QPainter(qimage)
            image_label = QLabel()
            image_label.setPixmap(data)
            image_label.render(painter, QPoint(0, 0), QRegion(0, 0, 128, 128))
            painter.end()
            rendered_tempfile = str(tmp_path / f"temp_rendered_{mode}.png")
            qimage.save(rendered_tempfile)
            assert_image_equal_tofile(im.convert("RGBA"), rendered_tempfile)
    
            # from QPixmap
>           roundtrip(hopper(mode))

Tests/test_qt_image_qapplication.py:89: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
Tests/test_qt_image_qapplication.py:51: in roundtrip
    assert_image_equal(result, expected.convert("RGB"))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

a = <PIL.PpmImagePlugin.PpmImageFile image mode=RGB size=128x128 at 0xC18DA18>
b = <PIL.Image.Image image mode=RGB size=128x128 at 0xC18DC70>, msg = None

    def assert_image_equal(a, b, msg=None):
        assert a.mode == b.mode, msg or f"got mode {repr(a.mode)}, expected {repr(b.mode)}"
        assert a.size == b.size, msg or f"got size {repr(a.size)}, expected {repr(b.size)}"
        if a.tobytes() != b.tobytes():
            if HAS_UPLOADER:
                try:
                    url = test_image_results.upload(a, b)
                    logger.error(f"Url for test images: {url}")
                except Exception:
                    pass
    
>           assert False, msg or "got different content"
E           AssertionError: got different content
E           assert False

Tests/helper.py:98: AssertionError

The logs

Here's two failures from a single PR:

The test

@pytest.mark.skipif(not ImageQt.qt_is_installed, reason="Qt bindings are not installed")
def test_sanity(tmp_path):
# Segfault test
app = QApplication([])
ex = Example()
assert app # Silence warning
assert ex # Silence warning
for mode in ("1", "RGB", "RGBA", "L", "P"):
# to QPixmap
im = hopper(mode)
data = ImageQt.toqpixmap(im)
assert isinstance(data, QPixmap)
assert not data.isNull()
# Test saving the file
tempfile = str(tmp_path / f"temp_{mode}.png")
data.save(tempfile)
# Render the image
qimage = ImageQt.ImageQt(im)
data = QPixmap.fromImage(qimage)
qt_format = QImage.Format if ImageQt.qt_version == "6" else QImage
qimage = QImage(128, 128, qt_format.Format_ARGB32)
painter = QPainter(qimage)
image_label = QLabel()
image_label.setPixmap(data)
image_label.render(painter, QPoint(0, 0), QRegion(0, 0, 128, 128))
painter.end()
rendered_tempfile = str(tmp_path / f"temp_rendered_{mode}.png")
qimage.save(rendered_tempfile)
assert_image_equal_tofile(im.convert("RGBA"), rendered_tempfile)
# from QPixmap
roundtrip(hopper(mode))
app.quit()
app = None

@radarhere
Copy link
Member

radarhere commented Jan 28, 2023

I ran the test suite repeatedly in GitHub Actions, and managed to have the roundtrip() check fail during the RGB iteration of the loop. Here is what the image looks like when it fails.

result

There are three short lines of incorrect pixels in the image. If that is acceptable, then PR #6915 relaxes the check so that this image would have passed.

However, it's not just during roundtrip() that there is a problem - https://github.com/python-pillow/Pillow/actions/runs/3952817049/jobs/6768999113 has a failure that occurs two lines earlier in assert_image_equal_tofile().

@hugovk
Copy link
Member Author

hugovk commented Jan 28, 2023

Thank you, #6915 merged!

@radarhere
Copy link
Member

However, it's not just during roundtrip() that there is a problem - https://github.com/python-pillow/Pillow/actions/runs/3952817049/jobs/6768999113 has a failure that occurs two lines earlier in assert_image_equal_tofile().

I've run our test suite in Ubuntu on GitHub Actions several thousand times since the PR was merged, and haven't seen any further errors. Maybe this assert_image_equal_tofile() problem is very rare.

I think if we wait a while, and this hasn't re-occurred, then this can be closed.

This was referenced Feb 23, 2023
@radarhere
Copy link
Member

It's been a month since #6970 was merged, and I haven't seen any of these failures since.

I think this can be closed.

@hugovk
Copy link
Member Author

hugovk commented Mar 24, 2023

Agreed, thank you very much!

@hugovk hugovk closed this as completed Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants