|
13 | 13 |
|
14 | 14 | # Preventing segfaults in test suite that has Qt Tests
|
15 | 15 |
|
| 16 | +[Updated 2029.10.09 with information about "Detect leaked widgets"] |
| 17 | + |
16 | 18 | ## Motivation
|
17 | 19 |
|
18 | 20 | When providing an GUI application one needs to select GUI backend.
|
@@ -228,12 +230,170 @@ For other problematic objects you can use similar approach. There are proper fix
|
228 | 230 |
|
229 | 231 |
|
230 | 232 | ### Detect leaked widgets
|
231 |
| - |
232 |
| -TBA https://github.com/napari/napari/pull/7251 |
| 233 | + |
| 234 | +!!! note |
| 235 | + If your test suite is small it may be much simpler to review all tests and check if all top level widgets are scheduled for deletion. |
| 236 | + |
| 237 | +With big test dataset it may be hard to detect if some widget is not scheduled for delete. |
| 238 | + |
| 239 | +This whole section is describing set of heuristics that may help to detect such widgets, but may also lead to false positives. |
| 240 | +If you use some custom, complex procedure for widget deletion you may need to adjust these heuristics or meet strange errors. |
| 241 | +This heuristic may report some widget after many test suite runs. It means that in previous test suite runs this widget was deleted by garbage collector, but in this run it was not. |
| 242 | + |
| 243 | +!!! note |
| 244 | + If you are not expert in Qt and Python I strongly suggest to not write custom teardown procedure for widgets |
| 245 | + and just use `qtbot.add_widget` method everywhere. |
| 246 | + |
| 247 | +#### `QApplication.topLevelWidgets` |
| 248 | + |
| 249 | +The Qt provides method [`QApplication.topLevelWidgets`](https://doc.qt.io/qt-6/qapplication.html#topLevelWidgets) that returns list of all top level widgets. |
| 250 | +It is nice place to start searching for leaked widgets. Hoverer it has some limitations: |
| 251 | + |
| 252 | +1. It may create new python wrappers for widgets, so all methods that are monkeypatched or properties defined outside `__init__` method may not be available. |
| 253 | +2. Not all top level widgets are top level widgets that require teardown setup. For example, it returns `QMenu` objects that represents the main window menu bar. |
| 254 | +3. It returns all top level widgets, not only those that are created in test. |
| 255 | + |
| 256 | + |
| 257 | +Based on above info we cannot use custom attribute to mark widget as handled without defining them in constructor. |
| 258 | +However, all Qt Objects have `objectName` property that stored in C++ object and is not recreated in python wrapper. |
| 259 | +But it is also could be used by custom code or styling, so it is not perfect. |
| 260 | + |
| 261 | +For code simplicity we will use `objectName` property to mark handled widgets. |
| 262 | +We will do this by subleasing `QtBot` class from `pytest-qt` plugin and overriding `addWidget` method. |
| 263 | + |
| 264 | +We will use fact that `qtbot.addWidget` allow for add custom teardown function that will be called before widget is deleted. |
| 265 | +It is done by providing `before_close_func` argument to `addWidget` method. So if object added to `qtbot` |
| 266 | +have `objectName` set to some value it could be changed in `before_close_func` function. |
| 267 | + |
| 268 | +We also need to define own `qtbot` fixture that will use our custom `QtBot` class. |
| 269 | + |
| 270 | +```python |
| 271 | +from pytestqt.qtbot import QtBot |
| 272 | + |
| 273 | +class QtBotWithOnCloseRenaming(QtBot): |
| 274 | + """Modified QtBot that renames widgets when closing them in tests. |
| 275 | +
|
| 276 | + After a test ends that uses QtBot, all instantiated widgets added to |
| 277 | + the bot have their name changed to 'handled_widget'. This allows us to |
| 278 | + detect leaking widgets at the end of a test run, and avoid the |
| 279 | + segmentation faults that often result from such leaks. [1]_ |
| 280 | +
|
| 281 | + See Also |
| 282 | + -------- |
| 283 | + `_find_dangling_widgets`: fixture that finds all widgets that have not |
| 284 | + been renamed to 'handled_widget'. |
| 285 | +
|
| 286 | + References |
| 287 | + ---------- |
| 288 | + .. [1] https://czaki.github.io/blog/2024/09/16/preventing-segfaults-in-test-suite-that-has-qt-tests/ |
| 289 | + """ |
| 290 | + |
| 291 | + def addWidget(self, widget, *, before_close_func=None): |
| 292 | + # in QtBot implementation, the `add_widget` method is just calling `addWidget` |
| 293 | + if widget.objectName() == '': |
| 294 | + # object does not have a name, so we can set it |
| 295 | + widget.setObjectName('handled_widget') |
| 296 | + before_close_func_ = before_close_func |
| 297 | + elif before_close_func is None: |
| 298 | + # there is no custom teardown function, |
| 299 | + # so we provide one that will set object name |
| 300 | + |
| 301 | + def before_close_func_(w): |
| 302 | + w.setObjectName('handled_widget') |
| 303 | + else: |
| 304 | + # user provided custom teardown function, |
| 305 | + # so we need to wrap it to set object name |
| 306 | + |
| 307 | + def before_close_func_(w): |
| 308 | + before_close_func(w) |
| 309 | + w.setObjectName('handled_widget') |
| 310 | + |
| 311 | + super().addWidget(widget, before_close_func=before_close_func_) |
| 312 | + |
| 313 | + |
| 314 | +@pytest.fixture |
| 315 | +def qtbot(qapp, request): # pragma: no cover |
| 316 | + """Fixture to create a QtBotWithOnCloseRenaming instance for testing. |
| 317 | +
|
| 318 | + Make sure to call addWidget for each top-level widget you create to |
| 319 | + ensure that they are properly closed after the test ends. |
| 320 | +
|
| 321 | + The `qapp` fixture is used to ensure that the QApplication is created |
| 322 | + before, so we need it, even without using it directly in this fixture. |
| 323 | + """ |
| 324 | + return QtBotWithOnCloseRenaming(request) |
| 325 | +``` |
| 326 | + |
| 327 | +!!! note |
| 328 | + As I expect that many readers of this blog post may be maintainers of napari plugins, |
| 329 | + the code bellow contains parts specific to the napari project. They are marked with a comment. |
| 330 | + If you are not a napari plugin maintainer, you can remove these parts. |
| 331 | + |
| 332 | +The bellow fixture is implementing our heuristic to detect leaked widgets. |
| 333 | +It looks for all top level widgets that are not children of any other widget and have not been renamed to `handled_widget`. |
| 334 | +Then raises an exception with a list of such widgets. |
| 335 | + |
| 336 | + |
| 337 | +```python |
| 338 | +@pytest.fixture(autouse=True) |
| 339 | +def _find_dangling_widgets(request, qtbot): |
| 340 | + yield |
| 341 | + |
| 342 | + from qtpy.QtWidgets import QApplication |
| 343 | + |
| 344 | + from napari._qt.qt_main_window import _QtMainWindow |
| 345 | + |
| 346 | + top_level_widgets = QApplication.topLevelWidgets() |
| 347 | + |
| 348 | + viewer_weak_set = getattr(request.node, '_viewer_weak_set', set()) |
| 349 | + # viewer_weak_set is used to store weak references to napari viewers |
| 350 | + # it is required if you use `make_napari_viewer` fixture in your tests |
| 351 | + |
| 352 | + problematic_widgets = [] |
| 353 | + |
| 354 | + for widget in top_level_widgets: |
| 355 | + if widget.parent() is not None: |
| 356 | + # if it has a parent, then it is enough to schedule the parent for deletion |
| 357 | + continue |
| 358 | + if ( |
| 359 | + isinstance(widget, _QtMainWindow) |
| 360 | + and widget._qt_viewer.viewer in viewer_weak_set |
| 361 | + ): |
| 362 | + # this if is for napari viewer created using |
| 363 | + # make_napari_viewer fixture |
| 364 | + continue |
| 365 | + |
| 366 | + if widget.__class__.__module__.startswith('qtconsole'): |
| 367 | + # this is for jupyter qtconsole |
| 368 | + # we do not found yet how to properly handle some of widgets in it |
| 369 | + continue |
| 370 | + |
| 371 | + if widget.objectName() == 'handled_widget': |
| 372 | + continue |
| 373 | + |
| 374 | + problematic_widgets.append(widget) |
| 375 | + |
| 376 | + if problematic_widgets: |
| 377 | + text = '\n'.join( |
| 378 | + f'Widget: {widget} of type {type(widget)} with name {widget.objectName()}' |
| 379 | + for widget in problematic_widgets |
| 380 | + ) |
| 381 | + |
| 382 | + for widget in problematic_widgets: |
| 383 | + # we set here object name to not raise exception in next test |
| 384 | + widget.setObjectName('handled_widget') |
| 385 | + |
| 386 | + raise RuntimeError(f'Found dangling widgets:\n{text}') |
| 387 | + |
| 388 | +``` |
| 389 | +' |
| 390 | +<!-- TBA https://github.com/napari/napari/pull/7251 --> |
233 | 391 |
|
234 | 392 |
|
235 | 393 | ## Bonus tip
|
236 | 394 |
|
| 395 | +### Test hanging due to nested event loop |
| 396 | + |
237 | 397 | Your tests are hanging, but any above solution did not help. What to do?
|
238 | 398 |
|
239 | 399 | One of the possible reason is that your code is created some nested event loop by opening [`QDialog`](https://doc.qt.io/qt-6/qdialog.html)
|
|
0 commit comments