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

ModelTester for recursive tree models #501

Open
phil65 opened this issue Jul 1, 2023 · 5 comments
Open

ModelTester for recursive tree models #501

phil65 opened this issue Jul 1, 2023 · 5 comments

Comments

@phil65
Copy link

phil65 commented Jul 1, 2023

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!

@The-Compiler
Copy link
Member

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
Copy link
Member

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)
 
 
+@pytest.mark.parametrize("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...

@phil65
Copy link
Author

phil65 commented Aug 9, 2023

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

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

@The-Compiler
Copy link
Member

The-Compiler commented Aug 9, 2023

That's just syntactic sugar for:

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

so it shows the same problem as well.

@phil65
Copy link
Author

phil65 commented Aug 10, 2023

Okay. Thanks for trying!

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

No branches or pull requests

2 participants