From 22d65e047ac75fc3527ddac2401e90a2805a6f04 Mon Sep 17 00:00:00 2001 From: Sebastian Englbrecht Date: Sat, 23 May 2026 13:27:36 +0200 Subject: tests: replace QTest::qWait(N) with signal-based waits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed-time sleeps are a source of intermittent failures on slow CI machines — if the delay is too short, the test races ahead of the code under test. Replacements made: - dolphinmainwindowtest: qWait() calls replaced with QTRY_COMPARE, QTRY_VERIFY, qWaitFor lambdas and processEvents() where animations are disabled; one genuinely unavoidable timer wait marked // UNAVOIDABLE: - kitemlistcontrollerexpandtest: qWaitFor(..., 100ms) / qWaitFor(..., 300ms) replaced with spy->wait() loops that block until all expected directoryLoadingCompleted signals have arrived - kitemlistkeyboardsearchmanagertest: timer-driven wait marked // UNAVOIDABLE: (must wait for keyboard search timeout to expire) Add testhelpers.h providing TestHelpers::disableAnimations(), which eliminates the need for delays that previously waited for Qt UI animations to settle. --- src/tests/dolphinmainwindowtest.cpp | 22 ++++++++++---------- src/tests/kitemlistcontrollerexpandtest.cpp | 26 ++++++++++-------------- src/tests/kitemlistkeyboardsearchmanagertest.cpp | 2 +- src/tests/testhelpers.h | 26 ++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 27 deletions(-) create mode 100644 src/tests/testhelpers.h diff --git a/src/tests/dolphinmainwindowtest.cpp b/src/tests/dolphinmainwindowtest.cpp index 21e48f246..24d2025b7 100644 --- a/src/tests/dolphinmainwindowtest.cpp +++ b/src/tests/dolphinmainwindowtest.cpp @@ -37,6 +37,8 @@ #include #include +#include "testhelpers.h" + #include #include @@ -81,6 +83,7 @@ private: void DolphinMainWindowTest::initTestCase() { QStandardPaths::setTestModeEnabled(true); + TestHelpers::disableAnimations(); // Use fullWidth statusbar during testing, to test out most of the features. GeneralSettings *settings = GeneralSettings::self(); settings->setShowStatusBar(GeneralSettings::EnumShowStatusBar::FullWidth); @@ -585,11 +588,11 @@ void DolphinMainWindowTest::testPlacesPanelWidthResistance() }, 5000), "The test couldn't be initialised properly. The places panel should be visible."); - QTest::qWait(100); + QApplication::processEvents(); const int initialPlacesPanelWidth = placesPanel->width(); m_mainWindow->actionCollection()->action(QStringLiteral("split_view"))->trigger(); // enable split view (starts animation) - QTest::qWait(300); // wait for animation + QApplication::processEvents(); QCOMPARE(placesPanel->width(), initialPlacesPanelWidth); m_mainWindow->actionCollection()->action(QStringLiteral("show_filter_bar"))->trigger(); @@ -602,7 +605,7 @@ void DolphinMainWindowTest::testPlacesPanelWidthResistance() selectionModeStates++) { const auto contents = static_cast(selectionModeStates); m_mainWindow->slotSetSelectionMode(true, contents); - QTest::qWait(20); // give time for a paint/resize + QApplication::processEvents(); // give time for a paint/resize QCOMPARE(placesPanel->width(), initialPlacesPanelWidth); } @@ -635,7 +638,7 @@ void DolphinMainWindowTest::testPlacesPanelWidthResistance() m_mainWindow->showMaximized(); QCOMPARE(placesPanel->width(), initialPlacesPanelWidth); - QTest::qWait(300); // wait for split view closing animation + QApplication::processEvents(); // animations disabled via disableAnimations() in initTestCase() QCOMPARE(placesPanel->width(), initialPlacesPanelWidth); } @@ -664,7 +667,6 @@ void DolphinMainWindowTest::testGoActions() QVERIFY(QTest::qWaitFor([&]() { return !m_mainWindow->actionCollection()->action(QStringLiteral("stop"))->isEnabled(); })); // "Stop" command should be disabled because it finished loading - QTest::qWait(500); // Somehow the item we emerged from doesn't have keyboard focus yet if we don't wait a split second. const QUrl parentDirUrl = m_mainWindow->activeViewContainer()->url(); QVERIFY(parentDirUrl != childDirUrl); @@ -674,7 +676,7 @@ void DolphinMainWindowTest::testGoActions() return currentItem.url(); }; - QCOMPARE(currentItemUrl(), childDirUrl); // The item we just emerged from should now have keyboard focus. + QTRY_COMPARE(currentItemUrl(), childDirUrl); // The item we just emerged from should now have keyboard focus. QCOMPARE(m_mainWindow->m_activeViewContainer->view()->selectedItems().count(), 1); // …and it should be selected, too. // Pressing arrow keys should not only move the keyboard focus but also select the item. // We press "Down" to select "c" below and then "Up" so the folder "b" we just emerged from is selected for the first time. @@ -695,11 +697,10 @@ void DolphinMainWindowTest::testGoActions() // Go back to the parent folder. m_mainWindow->actionCollection()->action(KStandardAction::name(KStandardAction::Back))->trigger(); QVERIFY(spyDirectoryLoadingCompleted.wait()); - QTest::qWait(100); // Somehow the item we emerged from doesn't have keyboard focus yet if we don't wait a split second. QCOMPARE(m_mainWindow->activeViewContainer()->url(), parentDirUrl); QVERIFY(m_mainWindow->isUrlOpen(parentDirUrl.toString())); // Going 'Back' means that the view should be in the same state it was in when we left. - QCOMPARE(currentItemUrl(), childDirUrl); // The item we last interacted with in this location should still have keyboard focus. + QTRY_COMPARE(currentItemUrl(), childDirUrl); // The item we last interacted with in this location should still have keyboard focus. QCOMPARE(m_mainWindow->m_activeViewContainer->view()->selectedItems().count(), 1); QCOMPARE(m_mainWindow->m_activeViewContainer->view()->selectedItems().constFirst().url(), childDirUrl); // It should still be selected. @@ -739,7 +740,6 @@ void DolphinMainWindowTest::testGoActions() // Go back to the parent folder. m_mainWindow->actionCollection()->action(KStandardAction::name(KStandardAction::Back))->trigger(); QVERIFY(spyDirectoryLoadingCompleted.wait()); - QTest::qWait(100); // Somehow the item we emerged from doesn't have keyboard focus yet if we don't wait a split second. QCOMPARE(m_mainWindow->activeViewContainer()->url(), parentDirUrl); QVERIFY(m_mainWindow->isUrlOpen(parentDirUrl.toString())); @@ -993,7 +993,7 @@ void DolphinMainWindowTest::testInlineRename() QSignalSpy modelDirectoryLoadingCompletedSpy(view->m_model, &KFileItemModel::directoryLoadingCompleted); QVERIFY(viewDirectoryLoadingCompletedSpy.wait()); - QTest::qWait(500); // we need to wait for the file widgets to become visible + QTest::qWait(500); // UNAVOIDABLE: view must be fully settled before inline rename sequence view->markUrlsAsSelected({QUrl(testDir->url().toString() + "/aaaa")}); view->updateViewState(); view->renameSelectedItems(); @@ -1059,7 +1059,7 @@ void DolphinMainWindowTest::testThumbnailAfterRename() QVERIFY(previewUpdatedSpy.wait()); QVERIFY(QTest::qWaitForWindowExposed(m_mainWindow.data())); QVERIFY(m_mainWindow->isVisible()); - QTest::qWait(500); // we need to wait for the file widgets to become visible + QTRY_COMPARE(view->m_view->m_visibleItems.count(), view->m_model->count()); // wait for all file widgets to be laid out // Set image selected and rename it to b.jpg, make sure editing role is working view->markUrlsAsSelected({QUrl(testDir->url().toString() + "/a.jpg")}); diff --git a/src/tests/kitemlistcontrollerexpandtest.cpp b/src/tests/kitemlistcontrollerexpandtest.cpp index 8f16251b4..cb84fb693 100644 --- a/src/tests/kitemlistcontrollerexpandtest.cpp +++ b/src/tests/kitemlistcontrollerexpandtest.cpp @@ -152,11 +152,8 @@ void KItemListControllerExpandTest::testDirExpand() // expand selected folders QTest::keyClick(m_container, Qt::Key_Right); - QVERIFY(QTest::qWaitFor( - [this]() { - return m_spyDirectoryLoadingCompleted->count() == 3; - }, - 100)); + QVERIFY(m_spyDirectoryLoadingCompleted->wait()); + QCOMPARE(m_spyDirectoryLoadingCompleted->count(), 3); QCOMPARE(m_model->count(), 8); QCOMPARE(m_selectionManager->currentItem(), 6); QCOMPARE(m_selectionManager->selectedItems().count(), 2); @@ -179,11 +176,11 @@ void KItemListControllerExpandTest::testDirExpand() // expand the three folders QTest::keyClick(m_container, Qt::Key_Right); - QVERIFY(QTest::qWaitFor( - [this]() { - return m_spyDirectoryLoadingCompleted->count() == 6; - }, - 300)); + // Key_Right expands all 3 selected folders, each triggering one directoryLoadingCompleted. + // spy->wait() unblocks on a single emission, so loop until all 3 have arrived. + while (m_spyDirectoryLoadingCompleted->count() < 6) { + QVERIFY(m_spyDirectoryLoadingCompleted->wait()); + } QCOMPARE(m_model->count(), 18); QCOMPARE(m_selectionManager->currentItem(), 12); @@ -216,11 +213,10 @@ void KItemListControllerExpandTest::testDirExpand() // expand the three folders with shift modifier QTest::keyClick(m_container, Qt::Key_Right, Qt::ShiftModifier); - QVERIFY(QTest::qWaitFor( - [this]() { - return m_spyDirectoryLoadingCompleted->count() == 9; - }, - 100)); + // Same as above: 3 folders expand in parallel, one signal each — loop until all 3 arrive. + while (m_spyDirectoryLoadingCompleted->count() < 9) { + QVERIFY(m_spyDirectoryLoadingCompleted->wait()); + } QCOMPARE(m_model->count(), 18); QCOMPARE(m_selectionManager->currentItem(), 12); diff --git a/src/tests/kitemlistkeyboardsearchmanagertest.cpp b/src/tests/kitemlistkeyboardsearchmanagertest.cpp index feba6acba..601576102 100644 --- a/src/tests/kitemlistkeyboardsearchmanagertest.cpp +++ b/src/tests/kitemlistkeyboardsearchmanagertest.cpp @@ -137,7 +137,7 @@ void KItemListKeyboardSearchManagerTest::testAbortedKeyboardSearch() // If the delay between two key presses is larger than the chosen timeout, // a new search is started. We add a small safety margin to avoid race conditions. - QTest::qWait(m_keyboardSearchManager.timeout() + 10); + QTest::qWait(m_keyboardSearchManager.timeout() + 10); // UNAVOIDABLE: must wait for keyboard search timeout to expire m_keyboardSearchManager.addKeys("l"); verifySignal(spy, "l", true); diff --git a/src/tests/testhelpers.h b/src/tests/testhelpers.h new file mode 100644 index 000000000..e73f5449f --- /dev/null +++ b/src/tests/testhelpers.h @@ -0,0 +1,26 @@ +/* + * SPDX-FileCopyrightText: 2026 Sebastian Englbrecht + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#pragma once + +#include +#include + +namespace TestHelpers +{ + +/** + * Disables all Qt UI animations. + * Call once in initTestCase(). + * Eliminates the need for qWait(N) hacks that wait for animations to finish. + */ +inline void disableAnimations() +{ + QApplication::setEffectEnabled(Qt::UI_AnimateMenu, false); + QApplication::setEffectEnabled(Qt::UI_AnimateCombo, false); + QApplication::setEffectEnabled(Qt::UI_AnimateTooltip, false); + QApplication::setEffectEnabled(Qt::UI_AnimateToolBox, false); +} + +} // namespace TestHelpers -- cgit v1.3.1