From f124412896774633b6428ccaa03bdf099b8e9d8b Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Sat, 27 Feb 2016 22:17:07 +0100 Subject: [PATCH] refs #605, fixed unit test itself * threaded reader were normal members causing a crash when those were moved in their own thread * we need own event processing for unit test * Network request needs to be generated in main thread * don`t shutdown readers on QCoreApplication::aboutToQuit, let CApplication handle it * use CApplication in unit test (as in real world) Unrelated: * access global setup via application --- src/blackcore/application.cpp | 71 ++++++++++++++++--- src/blackcore/application.h | 64 +++++++++-------- src/blackcore/databaseauthentication.cpp | 14 +--- src/blackmisc/threadedreader.cpp | 4 +- tests/blackcore/main.cpp | 9 ++- tests/blackcore/testblackcoremain.cpp | 4 -- tests/blackcore/testnetwork.cpp | 2 + tests/blackcore/testnetwork.h | 8 +-- tests/blackcore/testreaders.cpp | 87 +++++++++++++----------- tests/blackcore/testreaders.h | 15 ++-- 10 files changed, 167 insertions(+), 111 deletions(-) diff --git a/src/blackcore/application.cpp b/src/blackcore/application.cpp index 284f2ffb9..c78875648 100644 --- a/src/blackcore/application.cpp +++ b/src/blackcore/application.cpp @@ -18,6 +18,7 @@ #include "blackmisc/project.h" #include "blackmisc/dbusserver.h" #include "blackmisc/registermetadata.h" +#include "blackmisc/threadutils.h" #include "blackmisc/network/networkutils.h" #include "blackmisc/simulation/aircraftmodellist.h" #include "blackmisc/verify.h" @@ -32,6 +33,7 @@ using namespace BlackMisc::Aviation; using namespace BlackMisc::Simulation; using namespace BlackMisc::Weather; using namespace BlackCore; +using namespace BlackCore::Data; BlackCore::CApplication *sApp = nullptr; // set by constructor @@ -96,6 +98,13 @@ namespace BlackCore return QCoreApplication::instance()->applicationName() + " " + CProject::version(); } + Data::CGlobalSetup CApplication::getGlobalSetup() const + { + const CSetupReader *r = this->m_setupReader.data(); + if (!r) { return CGlobalSetup(); } + return r->getSetup(); + } + bool CApplication::start(bool waitForStart) { if (!this->m_parsed) @@ -130,7 +139,9 @@ namespace BlackCore const QTime dieTime = QTime::currentTime().addMSecs(5000); while (QTime::currentTime() < dieTime && !this->m_started && !this->m_startUpCompleted) { + // Alternative: use QEventLoop, which seemed to make the scenario here more complex QCoreApplication::instance()->processEvents(QEventLoop::AllEvents, 250); + QThread::msleep(250); // avoid CPU loop overload by "infinite loop" } if (!this->m_startUpCompleted) { @@ -170,46 +181,75 @@ namespace BlackCore return this->m_webDataServices.data(); } - QNetworkReply *CApplication::getFromNetwork(const CUrl &url, const BlackMisc::CSlot &callback) + bool CApplication::isApplicationThread() const + { + return CThreadUtils::isCurrentThreadApplicationThread(); + } + + QNetworkReply *CApplication::getFromNetwork(const CUrl &url, const CSlot &callback) { if (this->m_shutdown) { return nullptr; } return getFromNetwork(url.toNetworkRequest(), callback); } - QNetworkReply *CApplication::getFromNetwork(const QNetworkRequest &request, const BlackMisc::CSlot &callback) + QNetworkReply *CApplication::getFromNetwork(const QNetworkRequest &request, const CSlot &callback) { if (this->m_shutdown) { return nullptr; } - QNetworkRequest r(request); - CNetworkUtils::ignoreSslVerification(r); QWriteLocker locker(&m_accessManagerLock); + Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread"); + if (QThread::currentThread() != this->m_accessManager.thread()) + { + QTimer::singleShot(0, this, [this, request, callback]() { this->getFromNetwork(request, callback); }); + return nullptr; // not yet started + } + + Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch"); + QNetworkRequest r(request); // no QObject + CNetworkUtils::ignoreSslVerification(r); QNetworkReply *reply = this->m_accessManager.get(r); if (callback) { - connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }); + connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }, Qt::QueuedConnection); } return reply; } - QNetworkReply *CApplication::postToNetwork(const QNetworkRequest &request, const QByteArray &data, const BlackMisc::CSlot &callback) + QNetworkReply *CApplication::postToNetwork(const QNetworkRequest &request, const QByteArray &data, const CSlot &callback) { if (this->m_shutdown) { return nullptr; } + QWriteLocker locker(&m_accessManagerLock); + Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread"); + if (QThread::currentThread() != this->m_accessManager.thread()) + { + QTimer::singleShot(0, this, [this, request, data, callback]() { this->postToNetwork(request, data, callback); }); + return nullptr; // not yet started + } + + Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch"); QNetworkRequest r(request); CNetworkUtils::ignoreSslVerification(r); - QWriteLocker locker(&m_accessManagerLock); QNetworkReply *reply = this->m_accessManager.post(r, data); if (callback) { - connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }); + connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }, Qt::QueuedConnection); } return reply; } - QNetworkReply *CApplication::postToNetwork(const QNetworkRequest &request, QHttpMultiPart *multiPart, const BlackMisc::CSlot &callback) + QNetworkReply *CApplication::postToNetwork(const QNetworkRequest &request, QHttpMultiPart *multiPart, const CSlot &callback) { if (this->m_shutdown) { return nullptr; } + QWriteLocker locker(&m_accessManagerLock); + Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread"); + if (QThread::currentThread() != this->m_accessManager.thread()) + { + QTimer::singleShot(0, this, [this, request, multiPart, callback]() { this->postToNetwork(request, multiPart, callback); }); + return nullptr; // not yet started + } + + Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch"); QNetworkRequest r(request); CNetworkUtils::ignoreSslVerification(r); - QWriteLocker locker(&m_accessManagerLock); QNetworkReply *reply = this->m_accessManager.post(r, multiPart); if (callback) { @@ -244,6 +284,16 @@ namespace BlackCore return QCoreApplication::arguments(); } + void CApplication::processEventsFor(int milliseconds) + { + const QTime end = QTime::currentTime().addMSecs(milliseconds); + while (QTime::currentTime() <= end) + { + QCoreApplication::processEvents(); + QThread::msleep(100); + } + } + bool CApplication::useContexts(const CCoreFacadeConfig &coreConfig) { Q_ASSERT_X(this->m_parsed, Q_FUNC_INFO, "Call this after parsing"); @@ -380,6 +430,7 @@ namespace BlackCore this->m_started = this->asyncWebAndContextStart(); } this->m_startUpCompleted = true; + emit this->startUpCompleted(this->m_started); } bool CApplication::asyncWebAndContextStart() diff --git a/src/blackcore/application.h b/src/blackcore/application.h index 76ab87643..76d8dfd68 100644 --- a/src/blackcore/application.h +++ b/src/blackcore/application.h @@ -71,31 +71,9 @@ namespace BlackCore //! Application name and version QString getApplicationNameAndVersion() const; - //! Start services, if not yet parsed call CApplication::parse - virtual bool start(bool waitForStart = true); - - //! Wait for stert by calling the event loop and waiting until everything is ready - bool waitForStart(); - - //! Request to get network reply + //! Global setup //! \threadsafe - QNetworkReply *getFromNetwork(const BlackMisc::Network::CUrl &url, - const BlackMisc::CSlot &callback); - - //! Request to get network reply - //! \threadsafe - QNetworkReply *getFromNetwork(const QNetworkRequest &request, - const BlackMisc::CSlot &callback); - - //! Post to network - //! \threadsafe - QNetworkReply *postToNetwork(const QNetworkRequest &request, const QByteArray &data, - const BlackMisc::CSlot &callback); - - //! Post to network - //! \threadsafe - QNetworkReply *postToNetwork(const QNetworkRequest &request, QHttpMultiPart *multiPart, - const BlackMisc::CSlot &callback); + BlackCore::Data::CGlobalSetup getGlobalSetup() const; //! Delete all cookies from cookier manager void deleteAllCookies(); @@ -112,6 +90,9 @@ namespace BlackCore //! Get the web data services CWebDataServices *getWebDataServices() const; + //! Currently running in application thread? + bool isApplicationThread() const; + //! Run event loop static int exec(); @@ -121,6 +102,9 @@ namespace BlackCore //! Similar to QCoreApplication::arguments static QStringList arguments(); + //! Process all events for some time + static void processEventsFor(int milliseconds); + // ----------------------- parsing ---------------------------------------- //! \name parsing of command line options @@ -197,10 +181,39 @@ namespace BlackCore //! Graceful shutdown virtual void gracefulShutdown(); + //! Start services, if not yet parsed call CApplication::parse + virtual bool start(bool waitForStart = true); + + //! Wait for stert by calling the event loop and waiting until everything is ready + bool waitForStart(); + + //! Request to get network reply + //! \threadsafe + QNetworkReply *getFromNetwork(const BlackMisc::Network::CUrl &url, + const BlackMisc::CSlot &callback); + + //! Request to get network reply + //! \threadsafe + QNetworkReply *getFromNetwork(const QNetworkRequest &request, + const BlackMisc::CSlot &callback); + + //! Post to network + //! \threadsafe + QNetworkReply *postToNetwork(const QNetworkRequest &request, const QByteArray &data, + const BlackMisc::CSlot &callback); + + //! Post to network + //! \threadsafe + QNetworkReply *postToNetwork(const QNetworkRequest &request, QHttpMultiPart *multiPart, + const BlackMisc::CSlot &callback); + signals: //! Setup syncronized void setupSyncronized(); + //! Startup has been completed + void startUpCompleted(bool success); + //! Facade started void coreFacadeStarted(); @@ -212,9 +225,6 @@ namespace BlackCore void ps_setupSyncronized(bool success); protected: - //! Constructor - CApplication(const QString &applicationName, QCoreApplication *app); - //! Display help message virtual void cmdLineHelpMessage(); diff --git a/src/blackcore/databaseauthentication.cpp b/src/blackcore/databaseauthentication.cpp index bf101f84d..3faf9a6db 100644 --- a/src/blackcore/databaseauthentication.cpp +++ b/src/blackcore/databaseauthentication.cpp @@ -66,17 +66,9 @@ namespace BlackCore QString query = params.toString(); const QNetworkRequest request(CNetworkUtils::getNetworkRequest(url, CNetworkUtils::PostUrlEncoded)); - QNetworkReply *r = sApp->postToNetwork(request, query.toUtf8(), { this, &CDatabaseAuthenticationService::ps_parseServerResponse}); - if (!r) - { - QString rm("Cannot send request to authentication server %1"); - msgs.push_back(CStatusMessage(cats, CStatusMessage::SeverityError, rm.arg(url.toQString()))); - } - else - { - QString rm("Sent request to authentication server %1"); - msgs.push_back(CStatusMessage(cats, CStatusMessage::SeverityInfo, rm.arg(url.toQString()))); - } + sApp->postToNetwork(request, query.toUtf8(), { this, &CDatabaseAuthenticationService::ps_parseServerResponse}); + QString rm("Sent request to authentication server %1"); + msgs.push_back(CStatusMessage(cats, CStatusMessage::SeverityInfo, rm.arg(url.toQString()))); return msgs; } diff --git a/src/blackmisc/threadedreader.cpp b/src/blackmisc/threadedreader.cpp index d12898daa..dea4401ec 100644 --- a/src/blackmisc/threadedreader.cpp +++ b/src/blackmisc/threadedreader.cpp @@ -20,9 +20,7 @@ namespace BlackMisc CContinuousWorker(owner, name), m_updateTimer(new QTimer(this)) { - bool c = connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &CThreadedReader::gracefulShutdown); - Q_ASSERT_X(c, Q_FUNC_INFO, "Connect failed"); - Q_UNUSED(c); + // void } qint64 CThreadedReader::lastModifiedMsSinceEpoch(QNetworkReply *nwReply) const diff --git a/tests/blackcore/main.cpp b/tests/blackcore/main.cpp index 78c0e7af4..46628cefb 100644 --- a/tests/blackcore/main.cpp +++ b/tests/blackcore/main.cpp @@ -15,6 +15,7 @@ */ #include "testblackcoremain.h" +#include "blackcore/application.h" #include #include @@ -23,9 +24,11 @@ using namespace BlackCoreTest; //! Starter for test cases int main(int argc, char *argv[]) { - QCoreApplication a(argc, argv); - Q_UNUSED(a); - + QCoreApplication qa(argc, argv); + Q_UNUSED(qa); + BlackCore::CApplication a; + a.addVatlibOptions(); + a.start(); return CBlackCoreTestMain::unitMain(argc, argv); } diff --git a/tests/blackcore/testblackcoremain.cpp b/tests/blackcore/testblackcoremain.cpp index 983b3eb6a..475f79768 100644 --- a/tests/blackcore/testblackcoremain.cpp +++ b/tests/blackcore/testblackcoremain.cpp @@ -21,10 +21,6 @@ namespace BlackCoreTest { - - /* - * Starting main, equivalent to QTEST_APPLESS_MAIN for multiple test classes. - */ int CBlackCoreTestMain::unitMain(int argc, char *argv[]) { int status = 0; diff --git a/tests/blackcore/testnetwork.cpp b/tests/blackcore/testnetwork.cpp index c190d3b2e..905177d22 100644 --- a/tests/blackcore/testnetwork.cpp +++ b/tests/blackcore/testnetwork.cpp @@ -84,6 +84,8 @@ namespace BlackCoreTest qDebug() << "DISCONNECTED"; }) .wait(10); + + QThread::msleep(250); // make sure the last debug messages are written } bool CTestNetwork::pingServer(const CServer &server) diff --git a/tests/blackcore/testnetwork.h b/tests/blackcore/testnetwork.h index ef70274c7..9596a1c1a 100644 --- a/tests/blackcore/testnetwork.h +++ b/tests/blackcore/testnetwork.h @@ -11,11 +11,8 @@ #define BLACKCORETEST_TESTNETWORK_H //! \cond PRIVATE_TESTS - -/*! - * \file - * \ingroup testblackcore - */ +//! \file +//! \ingroup testblackcore #include "blackcore/networkvatlib.h" #include "blackcore/data/globalsetup.h" @@ -24,7 +21,6 @@ namespace BlackCoreTest { - /*! * INetwork implementation classes tests */ diff --git a/tests/blackcore/testreaders.cpp b/tests/blackcore/testreaders.cpp index 41694c68d..781446d89 100644 --- a/tests/blackcore/testreaders.cpp +++ b/tests/blackcore/testreaders.cpp @@ -8,15 +8,11 @@ */ //! \cond PRIVATE_TESTS - -/*! - * \file - * \ingroup testblackcore - */ +//! \file +//! \ingroup testblackcore #include "testreaders.h" -#include "expect.h" -#include "blackcore/data/globalsetup.h" +#include "blackcore/application.h" #include "blackmisc/network/networkutils.h" #include "blackmisc/aviation/aircrafticaocode.h" #include "blackmisc/aviation/airlineicaocode.h" @@ -32,54 +28,67 @@ namespace BlackCoreTest { CTestReaders::CTestReaders(QObject *parent) : QObject(parent), - m_icaoReader(this), - m_modelReader(this) + m_icaoReader(new CIcaoDataReader(this)), + m_modelReader(new CModelDataReader(this)) { } + CTestReaders::~CTestReaders() + { + this->m_icaoReader->gracefulShutdown(); + this->m_modelReader->gracefulShutdown(); + } + void CTestReaders::readIcaoData() { - CUrl url(m_setup.get().dbIcaoReaderUrl()); + const CUrl url(sApp->getGlobalSetup().dbIcaoReaderUrl()); if (!this->pingServer(url)) { return; } - m_icaoReader.start(); - Expect e(&this->m_icaoReader); - EXPECT_UNIT(e) - .send(&CIcaoDataReader::readInBackgroundThread, CEntityFlags::AllIcaoEntities, QDateTime()) - .expect(&CIcaoDataReader::dataRead, [url]() + m_icaoReader->start(); + m_icaoReader->readInBackgroundThread(CEntityFlags::AllIcaoEntities, QDateTime()); + + // expect does not work here, as I receive multiple signals for all entities and read states + // I have to run my "own event loop" + for (int i = 0; i < 60; i++) { - qDebug() << "Read ICAO data from" << url.getFullUrl(); - }) - .wait(10); + CApplication::processEventsFor(500); + if (this->m_icaoReader->getAircraftIcaoCodesCount() > 0 && this->m_icaoReader->getAirlineIcaoCodesCount() > 0) + { + break; + } + } - QVERIFY2(this->m_icaoReader.getAircraftIcaoCodesCount() > 0, "No aircraft ICAOs"); - QVERIFY2(this->m_icaoReader.getAirlineIcaoCodesCount() > 0, "No airline ICAOs"); + QVERIFY2(this->m_icaoReader->getAircraftIcaoCodesCount() > 0, "No aircraft ICAOs"); + QVERIFY2(this->m_icaoReader->getAirlineIcaoCodesCount() > 0, "No airline ICAOs"); - CAircraftIcaoCode aircraftIcao(this->m_icaoReader.getAircraftIcaoCodes().front()); - CAirlineIcaoCode airlineIcao(this->m_icaoReader.getAirlineIcaoCodes().front()); - QVERIFY2(aircraftIcao.hasCompleteData(), "Missing data for aircraft ICAO"); - QVERIFY2(airlineIcao.hasCompleteData(), "Missing data for airline ICAO"); + const CAircraftIcaoCode aircraftIcao(this->m_icaoReader->getAircraftIcaoCodes().frontOrDefault()); + const CAirlineIcaoCode airlineIcao(this->m_icaoReader->getAirlineIcaoCodes().frontOrDefault()); + QVERIFY2(aircraftIcao.hasDesignator(), "Missing data for aircraft ICAO"); + QVERIFY2(airlineIcao.hasValidDesignator(), "Missing data for airline ICAO"); + + CApplication::processEventsFor(2500); // make sure events are processed } void CTestReaders::readModelData() { - CUrl url(m_setup.get().dbModelReaderUrl()); + const CUrl url(sApp->getGlobalSetup().dbModelReaderUrl()); if (!this->pingServer(url)) { return; } - m_modelReader.start(); - Expect e(&this->m_modelReader); - EXPECT_UNIT(e) - .send(&CModelDataReader::readInBackgroundThread, CEntityFlags::DistributorLiveryModel, QDateTime()) - .expect(&CModelDataReader::dataRead, [url]() + m_modelReader->start(); + m_modelReader->readInBackgroundThread(CEntityFlags::ModelEntity, QDateTime()); + + for (int i = 0; i < 120; i++) { - qDebug() << "Read model data " << url; - }) - .wait(10); + CApplication::processEventsFor(500); + if (this->m_modelReader->getModelsCount() > 0) + { + break; + } + } - QVERIFY2(this->m_modelReader.getDistributorsCount() > 0, "No distributors"); - QVERIFY2(this->m_modelReader.getLiveriesCount() > 0, "No liveries"); + QVERIFY2(this->m_modelReader->getModelsCount() > 0, "No models"); - CLivery livery(this->m_modelReader.getLiveries().front()); - CDistributor distributor(this->m_modelReader.getDistributors().front()); - QVERIFY2(livery.hasCompleteData(), "Missing data for livery"); - QVERIFY2(distributor.hasCompleteData(), "Missing data for distributor"); + const CAircraftModel model(m_modelReader->getModels().frontOrDefault()); + QVERIFY2(model.getLivery().hasCompleteData(), "Missing data for livery"); + + CApplication::processEventsFor(2500); // make sure events are processed } bool CTestReaders::pingServer(const CUrl &url) diff --git a/tests/blackcore/testreaders.h b/tests/blackcore/testreaders.h index 939ef9c5f..76cf80d0a 100644 --- a/tests/blackcore/testreaders.h +++ b/tests/blackcore/testreaders.h @@ -11,11 +11,8 @@ #define BLACKCORETEST_TESTREADERS_H //! \cond PRIVATE_TESTS - -/*! - * \file - * \ingroup testblackcore - */ +//! \file +//! \ingroup testblackcore #include "blackcore/networkvatlib.h" #include "blackcore/modeldatareader.h" @@ -35,6 +32,9 @@ namespace BlackCoreTest //! Constructor. explicit CTestReaders(QObject *parent = nullptr); + //! Destructor + ~CTestReaders(); + private slots: //! Read ICAO data void readIcaoData(); @@ -43,9 +43,8 @@ namespace BlackCoreTest void readModelData(); private: - BlackCore::CIcaoDataReader m_icaoReader; - BlackCore::CModelDataReader m_modelReader; - BlackMisc::CData m_setup {this}; //!< setup cache + BlackCore::CIcaoDataReader *m_icaoReader = nullptr; + BlackCore::CModelDataReader *m_modelReader = nullptr; //! Test if server is available static bool pingServer(const BlackMisc::Network::CUrl &url);