Leaking top level widgets about pytest-qt HOT 5 OPEN

Sorry for the radio silence, this got lost with all the Europython and pytest training stuff!

I feel like we should still aim to:

• Find out at least why things are segfaulting when we do this
• Make sure our own testsuite doesn't leak any widgets after tests (now it does apparently, and it's unclear to me why!)
• Perhaps even offer something that fails tests when any widgets are leaked after them?

So yeah. This won't be a priority for myself for the forseeable future I'm afraid, but I still think we should investigate some more at some point.

from pytest-qt.

Hi @tlandschoff-scale,

Indeed it is the user's responsibility to close the widgets, that's why there's the qtbot.addWidget method.

I failed to directly and explicitly delete/destroy those top level widgets (there is no usable delete/destroy method exposed in Python).

Just closing (via .close()) the top level widgets is enough to effectively delete them in my experience.

We do mention to use addWidget in the tutorial:

Registering widgets is not required, but recommended because it will ensure those widgets get properly closed after each test is done.

I also have seen crashes when one does not close/destroy things properly, which is unfortunate.

Hope this helps.

from pytest-qt.

Hi Bruno,

Indeed it is the user's responsibility to close the widgets

1. this does not appear to be particularly robust (one broken test can go unnoticed for a long time that way)
2. without qtbot (and qtbot.addWidget) I did not see these problems

Especially, my new test did only fail when adding e.g. this existing test from pytest-qt to the test run:

def test_stop(qtbot, timer):
    widget = qt_api.QtWidgets.QWidget()
    qtbot.addWidget(widget)

    with qtbot.waitExposed(widget):
        widget.show()

    timer.single_shot_callback(widget.close, 0)
    qtbot.stop()

Given that adding this test to the run makes the fault appear, what does this code to wrong then?

Just closing (via .close()) the top level widgets is enough to effectively delete them in my experience.

I tried (on our code base), they still stay alive when using qtbot.

I also have seen crashes when one does not close/destroy things properly, which is unfortunate.

Not the fault of pytest-qt, I consider PyQt and PySide quite fragile in that matter. You get the C++ behaviour "API misuse => crash" from Python that way, without of the tool support you got in C++. But I think that is out of scope here, I just would like better isolation between tests using pytest-qt.

Greetings, Torsten

from pytest-qt.

Out of curiosity, I ran the tests with this patch:

diff --git a/tests/conftest.py b/tests/conftest.py
index 6010c31..f3cec82 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -7,6 +7,13 @@ from pytestqt.qt_compat import qt_api
 pytest_plugins = "pytester"
 
 
+def pytest_runtest_logfinish(nodeid, location):
+    app = qt_api.QtWidgets.QApplication.instance()
+    if app is not None:
+        n = len(app.topLevelWidgets())
+        print(f"{nodeid}: {n} widgets in the balance.")
+
+
 @pytest.fixture
 def stop_watch():
     """

Turns out that pytest-qt's own tests keep up to 19 widgets lying around:

.tests/test_wait_signal.py::test_blockers_handle_exceptions[False-multiple]: 19 widgets in the balance.
.tests/test_wait_signal.py::test_blockers_handle_exceptions[False-callback]: 19 widgets in the balance.
.tests/test_wait_signal.py::test_wait_twice[True-True]: 0 widgets in the balance.

Probably, test_wait_twice actually deletes those because an actual event loop is run there.

Seems like pytest-qt uses deleteLater to clean up widgets after running a test case:

def _close_widgets(item):
    """
    Close all widgets registered in the pytest item.
    """
    widgets = getattr(item, "qt_widgets", None)
    if widgets:
        for w, before_close_func in item.qt_widgets:
            w = w()
            if w is not None:
                if before_close_func is not None:
                    before_close_func(w)
                w.close()
                w.deleteLater()
        del item.qt_widgets

Note the docs on deleteLater:

If the event loop is not running when this function is called (e.g. deleteLater() is called on an object before [QCoreApplication::exec](https://doc.qt.io/qt-5/qcoreapplication.html#exec)()), the object will be deleted once the event loop is started.

So maybe, the simple fix would be to bring up a new QEventLoop in that code to actually purge those deleted widgets.

Greetings, Torsten

from pytest-qt.

Hi Torsten,

Unfortunately changing pytest-qt to close all widgets implicitly now (using app.topLevelWidgets()) can have unintended consequences. I know because we did an experiment in our large code base, and we started to get crashes left and right. Do not get me wrong, I'm sure the problem is somewhere in our code, but my point is that changing this on pytest-qt is dangerous and will probably cause suites to fail/crash.

Note that you do not need to patch pytest-qt to get that behavior, you can instead implement it using an autouse fixture in your conftest.py file:

@pytest.fixture
def close_all_widgets():
    yield
    app = QApplication.instance()
    if app is not None:
         for w in app.topLevelWidgets():
             w.close()

But as I said, I'm hesitant to add support for this directly in pytest-qt. @The-Compiler any thoughts?

from pytest-qt.