From 6403d3afd868bbdffddc52d8be0a56081637adf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCri=20Valdmann?= Date: Wed, 16 Sep 2020 10:43:52 +0200 Subject: Use less pointers in tst_loadsignals Make QWebEngineView a member of tst_LoadSignals. Also stop recreating it for every test. Task-number: QTBUG-65223 Change-Id: I2ed7c12559e56e23302813eb6f33c1e26a9d8748 Reviewed-by: Kirill Burtsev --- tests/auto/widgets/loadsignals/tst_loadsignals.cpp | 151 +++++++++------------ .../tst_qwebenginedownloaditem.cpp | 10 +- tests/auto/widgets/util.h | 11 ++ 3 files changed, 74 insertions(+), 98 deletions(-) diff --git a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp index e31cf50ed..b577a5625 100644 --- a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp +++ b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp @@ -40,14 +40,9 @@ class tst_LoadSignals : public QObject { Q_OBJECT -public: - tst_LoadSignals(); - virtual ~tst_LoadSignals(); - public Q_SLOTS: void initTestCase(); void init(); - void cleanup(); private Q_SLOTS: void monotonicity(); @@ -59,40 +54,23 @@ private Q_SLOTS: void fileDownloadDoesNotTriggerLoadSignals_qtbug66661(); private: - QWebEngineView* view; - QScopedPointer loadStartedSpy; - QScopedPointer loadProgressSpy; - QScopedPointer loadFinishedSpy; + QWebEngineView view; + QSignalSpy loadStartedSpy{view.page(), &QWebEnginePage::loadStarted}; + QSignalSpy loadProgressSpy{view.page(), &QWebEnginePage::loadProgress}; + QSignalSpy loadFinishedSpy{view.page(), &QWebEnginePage::loadFinished}; }; -tst_LoadSignals::tst_LoadSignals() -{ -} - -tst_LoadSignals::~tst_LoadSignals() -{ -} - void tst_LoadSignals::initTestCase() { + view.resize(1024,768); + view.show(); } void tst_LoadSignals::init() { - view = new QWebEngineView(); - view->resize(1024,768); - view->show(); - loadStartedSpy.reset(new QSignalSpy(view->page(), &QWebEnginePage::loadStarted)); - loadProgressSpy.reset(new QSignalSpy(view->page(), &QWebEnginePage::loadProgress)); - loadFinishedSpy.reset(new QSignalSpy(view->page(), &QWebEnginePage::loadFinished)); -} - -void tst_LoadSignals::cleanup() -{ - loadFinishedSpy.reset(); - loadProgressSpy.reset(); - loadStartedSpy.reset(); - delete view; + loadStartedSpy.clear(); + loadProgressSpy.clear(); + loadFinishedSpy.clear(); } /** @@ -115,18 +93,17 @@ void tst_LoadSignals::loadStartedAndFinishedCount() QFETCH(QUrl, url); QFETCH(int, expectedLoadCount); - view->load(url); - QTRY_COMPARE(loadFinishedSpy->size(), expectedLoadCount); - bool loadSucceeded = (*loadFinishedSpy)[0][0].toBool(); - QVERIFY(loadSucceeded); + view.load(url); + QTRY_COMPARE(loadFinishedSpy.size(), expectedLoadCount); + QVERIFY(loadFinishedSpy[0][0].toBool()); // Wait for 10 seconds (abort waiting if another loadStarted or loadFinished occurs) - QTRY_LOOP_IMPL((loadStartedSpy->size() != expectedLoadCount) - || (loadFinishedSpy->size() != expectedLoadCount), 10000, 100); + QTRY_LOOP_IMPL((loadStartedSpy.size() != expectedLoadCount) + || (loadFinishedSpy.size() != expectedLoadCount), 10000, 100); // No further loadStarted should have occurred within this time - QCOMPARE(loadStartedSpy->size(), expectedLoadCount); - QCOMPARE(loadFinishedSpy->size(), expectedLoadCount); + QCOMPARE(loadStartedSpy.size(), expectedLoadCount); + QCOMPARE(loadFinishedSpy.size(), expectedLoadCount); } /** @@ -141,23 +118,22 @@ void tst_LoadSignals::monotonicity() }); QVERIFY(server.start()); - view->load(server.url("/loadprogress/main.html")); - QTRY_COMPARE(loadFinishedSpy->size(), 1); - bool loadSucceeded = (*loadFinishedSpy)[0][0].toBool(); - QVERIFY(loadSucceeded); + view.load(server.url("/loadprogress/main.html")); + QTRY_COMPARE(loadFinishedSpy.size(), 1); + QVERIFY(loadFinishedSpy[0][0].toBool()); // first loadProgress should have 0% progress - QCOMPARE(loadProgressSpy->takeFirst()[0].toInt(), 0); + QCOMPARE(loadProgressSpy.takeFirst()[0].toInt(), 0); // every loadProgress should have more progress than the one before int progress = 0; - for (auto item : *loadProgressSpy) { + for (const auto &item : loadProgressSpy) { QVERIFY(progress < item[0].toInt()); progress = item[0].toInt(); } // last loadProgress should have 100% progress - QCOMPARE(loadProgressSpy->last()[0].toInt(), 100); + QCOMPARE(loadProgressSpy.last()[0].toInt(), 100); } /** @@ -176,26 +152,23 @@ void tst_LoadSignals::secondLoadForError_WhenErrorPageEnabled_data() void tst_LoadSignals::secondLoadForError_WhenErrorPageEnabled() { QFETCH(bool, enabled); - view->settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, enabled); + view.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, enabled); int expectedLoadCount = (enabled ? 2 : 1); // RFC 2606 guarantees that this will never become a valid domain - view->load(QUrl("http://nonexistent.invalid")); - QTRY_COMPARE_WITH_TIMEOUT(loadFinishedSpy->size(), expectedLoadCount, 10000); - bool loadSucceeded = (*loadFinishedSpy)[0][0].toBool(); - QVERIFY(!loadSucceeded); - if (enabled) { - bool errorPageLoadSucceeded = (*loadFinishedSpy)[1][0].toBool(); - QVERIFY(errorPageLoadSucceeded); - } + view.load(QUrl("http://nonexistent.invalid")); + QTRY_COMPARE_WITH_TIMEOUT(loadFinishedSpy.size(), expectedLoadCount, 10000); + QVERIFY(!loadFinishedSpy[0][0].toBool()); + if (enabled) + QVERIFY(loadFinishedSpy[1][0].toBool()); // Wait for 10 seconds (abort waiting if another loadStarted or loadFinished occurs) - QTRY_LOOP_IMPL((loadStartedSpy->size() != expectedLoadCount) - || (loadFinishedSpy->size() != expectedLoadCount), 10000, 100); + QTRY_LOOP_IMPL((loadStartedSpy.size() != expectedLoadCount) + || (loadFinishedSpy.size() != expectedLoadCount), 10000, 100); // No further loadStarted should have occurred within this time - QCOMPARE(loadStartedSpy->size(), expectedLoadCount); - QCOMPARE(loadFinishedSpy->size(), expectedLoadCount); + QCOMPARE(loadStartedSpy.size(), expectedLoadCount); + QCOMPARE(loadFinishedSpy.size(), expectedLoadCount); } /** @@ -204,24 +177,22 @@ void tst_LoadSignals::secondLoadForError_WhenErrorPageEnabled() */ void tst_LoadSignals::loadAfterInPageNavigation_qtbug66869() { - view->load(QUrl("qrc:///resources/page3.html")); - QTRY_COMPARE(loadFinishedSpy->size(), 1); - bool loadSucceeded = (*loadFinishedSpy)[0][0].toBool(); - QVERIFY(loadSucceeded); + view.load(QUrl("qrc:///resources/page3.html")); + QTRY_COMPARE(loadFinishedSpy.size(), 1); + QVERIFY(loadFinishedSpy[0][0].toBool()); // page3 does an in-page navigation after 500ms QTest::qWait(2000); - loadFinishedSpy->clear(); - loadProgressSpy->clear(); - loadStartedSpy->clear(); + loadFinishedSpy.clear(); + loadProgressSpy.clear(); + loadStartedSpy.clear(); // second load - view->load(QUrl("qrc:///resources/page1.html")); - QTRY_COMPARE(loadFinishedSpy->size(), 1); - loadSucceeded = (*loadFinishedSpy)[0][0].toBool(); - QVERIFY(loadSucceeded); + view.load(QUrl("qrc:///resources/page1.html")); + QTRY_COMPARE(loadFinishedSpy.size(), 1); + QVERIFY(loadFinishedSpy[0][0].toBool()); // loadStarted and loadFinished should have been signalled - QCOMPARE(loadStartedSpy->size(), 1); + QCOMPARE(loadStartedSpy.size(), 1); // reminder that we still need to solve the core issue QFAIL("https://codereview.qt-project.org/#/c/222112/ only hides the symptom, the core issue still needs to be solved"); @@ -233,41 +204,41 @@ void tst_LoadSignals::loadAfterInPageNavigation_qtbug66869() */ void tst_LoadSignals::fileDownloadDoesNotTriggerLoadSignals_qtbug66661() { - view->load(QUrl("qrc:///resources/page4.html")); - QTRY_COMPARE(loadFinishedSpy->size(), 1); - bool loadSucceeded = (*loadFinishedSpy)[0][0].toBool(); - QVERIFY(loadSucceeded); + view.load(QUrl("qrc:///resources/page4.html")); + QTRY_COMPARE(loadFinishedSpy.size(), 1); + QVERIFY(loadFinishedSpy[0][0].toBool()); // allow the download QTemporaryDir tempDir; QVERIFY(tempDir.isValid()); QWebEngineDownloadItem::DownloadState downloadState = QWebEngineDownloadItem::DownloadRequested; - connect(view->page()->profile(), &QWebEngineProfile::downloadRequested, - [&downloadState, &tempDir](QWebEngineDownloadItem* item){ - connect(item, &QWebEngineDownloadItem::stateChanged, [&downloadState](QWebEngineDownloadItem::DownloadState newState){ - downloadState = newState; - }); - item->setDownloadDirectory(tempDir.path()); - item->accept(); - }); + ScopedConnection sc1 = + connect(view.page()->profile(), &QWebEngineProfile::downloadRequested, + [&downloadState, &tempDir](QWebEngineDownloadItem *item) { + connect(item, &QWebEngineDownloadItem::stateChanged, + [&downloadState](QWebEngineDownloadItem::DownloadState newState) { + downloadState = newState; + }); + item->setDownloadDirectory(tempDir.path()); + item->accept(); + }); // trigger the download link that becomes focused on page4 QTest::qWait(1000); - QTest::sendKeyEvent(QTest::Press, view->focusProxy(), Qt::Key_Return, QString("\r"), Qt::NoModifier); - QTest::sendKeyEvent(QTest::Release, view->focusProxy(), Qt::Key_Return, QString("\r"), Qt::NoModifier); + QTest::sendKeyEvent(QTest::Press, view.focusProxy(), Qt::Key_Return, QString("\r"), Qt::NoModifier); + QTest::sendKeyEvent(QTest::Release, view.focusProxy(), Qt::Key_Return, QString("\r"), Qt::NoModifier); // Wait for 10 seconds (abort waiting if another loadStarted or loadFinished occurs) - QTRY_LOOP_IMPL((loadStartedSpy->size() != 1) - || (loadFinishedSpy->size() != 1), 10000, 100); + QTRY_LOOP_IMPL((loadStartedSpy.size() != 1) + || (loadFinishedSpy.size() != 1), 10000, 100); // Download must have occurred QTRY_COMPARE(downloadState, QWebEngineDownloadItem::DownloadCompleted); // No further loadStarted should have occurred within this time - QCOMPARE(loadStartedSpy->size(), 1); - QCOMPARE(loadFinishedSpy->size(), 1); + QCOMPARE(loadStartedSpy.size(), 1); + QCOMPARE(loadFinishedSpy.size(), 1); } - QTEST_MAIN(tst_LoadSignals) #include "tst_loadsignals.moc" diff --git a/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp b/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp index 81329338c..ac38e2ed2 100644 --- a/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp +++ b/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp @@ -26,6 +26,8 @@ ** ****************************************************************************/ +#include "../util.h" + #include #include #include @@ -100,14 +102,6 @@ private: QSet m_finishedDownloads; }; -class ScopedConnection { -public: - ScopedConnection(QMetaObject::Connection connection) : m_connection(std::move(connection)) {} - ~ScopedConnection() { QObject::disconnect(m_connection); } -private: - QMetaObject::Connection m_connection; -}; - Q_DECLARE_METATYPE(tst_QWebEngineDownloadItem::UserAction) Q_DECLARE_METATYPE(tst_QWebEngineDownloadItem::FileAction) diff --git a/tests/auto/widgets/util.h b/tests/auto/widgets/util.h index cb58f4243..af0b9bf6f 100644 --- a/tests/auto/widgets/util.h +++ b/tests/auto/widgets/util.h @@ -42,6 +42,17 @@ #define TESTS_SOURCE_DIR "" #endif +// Disconnect signal on destruction. +class ScopedConnection +{ +public: + ScopedConnection(QMetaObject::Connection connection) : m_connection(std::move(connection)) { } + ~ScopedConnection() { QObject::disconnect(m_connection); } + +private: + QMetaObject::Connection m_connection; +}; + /** * Just like QSignalSpy but facilitates sync and async * signal emission. For example if you want to verify that -- cgit v1.2.3