pytest-qt icon indicating copy to clipboard operation
pytest-qt copied to clipboard

ModelTester for recursive tree models

Open phil65 opened this issue 2 years ago • 5 comments

I´m running into a recursive loop when using the ModelTester with recursive tree models. QAbstractItemModelTester gained an option to not call fetchMore with Qt6.4. ( https://doc.qt.io/qt-6/qabstractitemmodeltester.html#setUseFetchMore ) Since the _qt_tester attribute of ModelTester is only set after calling ModelTester.check(), there is no way to use that option. Would be nice if that option could get added. Thank you!

phil65 avatar Jul 01 '23 11:07 phil65

Makes sense to support that option (and backport the logic to our Python reimplementation), yep!

Relevant change: Use QAbstractItemModelTester or QFileSystemModel (Ie5d2e22f) · Gerrit Code Review

No other relevant changes in Qt so far:

df9d882d41 Port from container.count()/length() to size()
76b07b05f0 Doc: add missing "see also"
2f35653a30 Use QAbstractItemModelTester or QFileSystemModel
05fc3aef53 Use SPDX license identifiers
b6759ff81c QAbstractItemModelTester: Fix typos in debug output    # <- Python reimplementation is here

The-Compiler avatar Jul 03 '23 14:07 The-Compiler

I did some initial work on this:

diff --git i/src/pytestqt/modeltest.py w/src/pytestqt/modeltest.py
index b2df3fe..c3d0340 100644
--- i/src/pytestqt/modeltest.py
+++ w/src/pytestqt/modeltest.py
@@ -71,6 +71,7 @@ class ModelTester:
     def __init__(self, config):
         self._model = None
         self._fetching_more = None
+        self._use_fetch_more = True
         self._insert = None
         self._remove = None
         self._changing = []
@@ -95,7 +96,7 @@ def _modelindex_debug(self, index):
                 id(index),
             )
 
-    def check(self, model, force_py=False):
+    def check(self, model, force_py=False, use_fetch_more=True):
         """Runs a series of checks in the given model.
 
         Connect to all of the models signals.
@@ -106,6 +107,9 @@ def check(self, model, force_py=False):
         :param force_py:
           Force using the Python implementation, even if the C++ implementation
           is available.
+        :param use_fetch_more:
+          Set to ``False`` to disable using ``fetchMore()`` on the model. If
+          using the Qt implementation, this needs Qt 6.4 or newer.
         """
         assert model is not None
 
@@ -116,6 +120,10 @@ def check(self, model, force_py=False):
             self._qt_tester = qt_api.QtTest.QAbstractItemModelTester(
                 model, reporting_mode
             )
+            if not use_fetch_more:
+                # If using use_fetch_more=False with Qt < 6.4, this will raise
+                # an AttributeError, which we let propagate.
+                self._qt_tester.setUseFetchMore(False)
             self._debug("Using Qt C++ tester")
             return
 
@@ -123,6 +131,7 @@ def check(self, model, force_py=False):
 
         self._model = model
         self._fetching_more = False
+        self._use_fetch_more = use_fetch_more
         self._insert = []
         self._remove = []
         self._changing = []
@@ -943,6 +952,8 @@ def _has_children(self, parent=qt_api.QtCore.QModelIndex()):
 
     def _fetch_more(self, parent):
         """Call ``fetchMore`` on the model and set ``self._fetching_more``."""
+        if not self._use_fetch_more:
+            return
         self._fetching_more = True
         self._model.fetchMore(parent)
         self._fetching_more = False
diff --git i/tests/test_modeltest.py w/tests/test_modeltest.py
index 665f125..c32f8de 100644
--- i/tests/test_modeltest.py
+++ w/tests/test_modeltest.py
@@ -302,21 +302,41 @@ def rowCount(self, parent=None):
     assert model.row_count_did_run
 
 
-def test_fetch_more(qtmodeltester):
-    class Model(qt_api.QtGui.QStandardItemModel):
-        def canFetchMore(self, parent):
-            return True
+class FetchMoreModel(qt_api.QtGui.QStandardItemModel):
 
-        def fetchMore(self, parent):
-            """Force a re-check while fetching more."""
-            self.setData(self.index(0, 0), "bar")
+    def __init__(self, fetch_more_allowed=True, parent=None):
+        super().__init__(parent)
+        self._fetch_more_allowed = fetch_more_allowed
 
-    model = Model()
+    def canFetchMore(self, parent):
+        return True
+
+    def fetchMore(self, parent):
+        """Force a re-check while fetching more."""
+        assert self._fetch_more_allowed
+        self.setData(self.index(0, 0), "bar")
+
+
+def test_fetch_more(qtmodeltester):
+    model = FetchMoreModel()
     item = qt_api.QtGui.QStandardItem("foo")
     model.setItem(0, 0, item)
     qtmodeltester.check(model, force_py=True)
 
 
[email protected]("force_py", [True, False])
+def test_fetch_more_disabled(qtmodeltester, force_py):
+    model = FetchMoreModel(fetch_more_allowed=False)
+    item = qt_api.QtGui.QStandardItem("foo")
+    model.setItem(0, 0, item)
+    try:
+        # Will raise in the model if fetchMore is called
+        qtmodeltester.check(model, force_py=force_py, use_fetch_more=False)
+    except AttributeError as e:
+        # Qt tester on Qt < 6.4
+        pytest.skip(str(e))
+
+
 def test_invalid_parent(qtmodeltester):
     class Model(qt_api.QtGui.QStandardItemModel):
         def parent(self, index):

but the tests fail:


====================================================== FAILURES ======================================================
__________________________________________ test_fetch_more_disabled[False] ___________________________________________
CALL ERROR: Exceptions caught in Qt event loop:
________________________________________________________________________________
Traceback (most recent call last):
  File "/home/florian/proj/pytest-qt/tests/test_modeltest.py", line 316, in fetchMore
    assert self._fetch_more_allowed
AssertionError: assert False
 +  where False = <test_modeltest.FetchMoreModel object at 0x7f867950e7a0>._fetch_more_allowed
________________________________________________________________________________
Traceback (most recent call last):
  File "/home/florian/proj/pytest-qt/tests/test_modeltest.py", line 316, in fetchMore
    assert self._fetch_more_allowed
AssertionError: assert False
 +  where False = <test_modeltest.FetchMoreModel object at 0x7f867950e7a0>._fetch_more_allowed
________________________________________________________________________________
------------------------------------------------ Captured stdout call ------------------------------------------------
modeltest: Using Qt C++ tester

and I believe this is Qt's fault:

  • Given that useFetchMore is only a property, it follows that it needs to be set after creating the model tester and passing a model to it
  • However, passing a model to it immediately runs all tests, which calls nonDestructiveBasicTest
  • That in turn already calls fetchMore(), before we even had a chance to disable that

Not sure if I should report this as a Qt bug, or just adjust the test so it allows fetchMore to be called exactly once...

The-Compiler avatar Aug 08 '23 21:08 The-Compiler

Did you try setting the property in the constructor? QAbstractItemModelTester(model, useFetchMore=False)

(With Qt for Python, all properties can be set via constructor)

phil65 avatar Aug 09 '23 09:08 phil65

That's just syntactic sugar for:

tester = QAbstractItemModelTester(model)
tester.setUseFetchMore(False)

so it shows the same problem as well.

The-Compiler avatar Aug 09 '23 09:08 The-Compiler

Okay. Thanks for trying!

phil65 avatar Aug 10 '23 01:08 phil65