- Check if reload works at all
- Check if cursor positions are as they should
Details
Details
- Reviewers
dhaumann cullmann - Commits
- R39:f5f715ea1441: Add test for "Auto Reload Document" option
Diff Detail
Diff Detail
- Repository
- R39 KTextEditor
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
There is a getter bool KateGlobal::self()->unitTestMode(). You could use this in DocumentPrivate to reduce the timer from 3000 to e.g. 50ms or so to reduce testing time.
But besides that, this looks good to me, thanks for working on this!
Comment Actions
- Enhance the test by checks for proper cursor position
While adding your tiny change, it pointed out, that the test was not correct regarding needed waiting times. Pls compare new/old comments about the delays :-/ Any idea to the magic value?
I noticed..
- that slotDelayedHandleModOnHd is still called/processed. I'm not sure if that's OK
- KateViewInternal::updateView is very often called and so doc()->delayAutoReload() too. The intend was to block the reload while scrolling around. Perhaps could that blocking somewhere else better placed
Comment Actions
I think KateViewInternal::updateView is called for cursor blinking for instance. May that be an issue?
Comment Actions
Regarding ...
KateViewInternal::updateView is very often called
...I don't think so. It was much more frequently than some blinking. That observation was only a general notice, not to this test...IIRC
So, what do with this? Commit as it is?
Comment Actions
I think this can go in as is, as long as the test works determinstically, which I assume it does.
Comment Actions
I spend a couple of time for this stuff.
Would be nice someone else could try it or do some investigation with tools I'm not familiar with, and don't want to be atm. Without feedback I will push it in the next few days.
Comment Actions
@dhaumann asked elsewhere
Instead of waiting so long, we could alternatively also have a while loop waiting up to 20 times for 50ms. This way, the test would be faster if possible.
Like this?
diff --git a/autotests/src/katedocument_test.cpp b/autotests/src/katedocument_test.cpp index 4be9d7c3..e24a8ad9 100644 --- a/autotests/src/katedocument_test.cpp +++ b/autotests/src/katedocument_test.cpp @@ -655,14 +655,23 @@ void KateDocumentTest::testAutoReload() // without to wait before it doesn't work here. It mostly fails with 500, // it tend to work with 600, but is not guarantied to work even with 800 QTest::qWait(1000); + // No idea how to do some slice test here stream << "\nTest"; stream.flush(); - // Hardcoded delay m_modOnHdTimer in DocumentPrivate + // Try not to waste more time than needed, but we need to wait + const int timeSlice = 50; + // Hardcoded delay m_modOnHdTimer in DocumentPrivate // + min val with which it looks working reliable here - QTest::qWait(200 + 800); - QCOMPARE(doc.text(), "Hello\nTest"); + const int totalPatience = 200 + 800; + + QString refTxT = ("Hello\nTest"); + for (int w = 0; w <= totalPatience; w += timeSlice) { + QTest::qWait(timeSlice); + if (doc.text() == refTxT) break; + } + QCOMPARE(doc.text(), refTxT); // ...stay in the last line after reload! QCOMPARE(view->cursorPosition().line(), doc.documentEnd().line()); @@ -674,8 +683,12 @@ void KateDocumentTest::testAutoReload() stream.flush(); file.close(); - QTest::qWait(200 + 800); - QCOMPARE(doc.text(), "Hello\nTest\nWorld!"); + refTxT = ("Hello\nTest\nWorld!"); + for (int w = 0; w <= totalPatience; w += timeSlice) { + QTest::qWait(timeSlice); + if (doc.text() == refTxT) break; + } + QCOMPARE(doc.text(), refTxT); // ...and ensure we have not move around QCOMPARE(view->cursorPosition(), c); }
No Patch
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 3152ms
With Patch
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 2619ms 533ms saved
But it can still fail now :-(
Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted, 3127ms
also with increased waiting times
Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted, 3787ms Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted, 4802ms
Does maybe the check "if (doc.text() == " disturbs the nice waiting?
Playing with longer slice shows an effect around a 500ms slice, but then we have no benefit
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 3147ms
Comment Actions
Yes, that's exactly what I meant. But if it's not stable, then it does not help. Maybe in that case keep it as is for now?