From b8583a1e59943a4269a38df6f7e033aedebd0a8a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 1 Aug 2021 22:07:51 +0200 Subject: [PATCH 01/17] [libcalamares] Expand the number of URLs to check for connectivity - introduce a list of URLs instead of just one - ping each of them, in turn, until one responds --- src/libcalamares/network/Manager.cpp | 40 ++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 79db1a587..e5277d74e 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -27,7 +27,7 @@ namespace Network void RequestOptions::applyToRequest( QNetworkRequest* request ) const { -#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0) +#if QT_VERSION < QT_VERSION_CHECK( 5, 15, 0 ) constexpr const auto RedirectPolicyAttribute = QNetworkRequest::FollowRedirectsAttribute; #else constexpr const auto RedirectPolicyAttribute = QNetworkRequest::RedirectPolicyAttribute; @@ -60,8 +60,9 @@ public slots: void cleanupNam(); public: - QUrl m_hasInternetUrl; - bool m_hasInternet; + QVector< QUrl > m_hasInternetUrls; + bool m_hasInternet = false; + int m_lastCheckedUrlIndex = -1; Private(); @@ -155,8 +156,33 @@ Manager::hasInternet() bool Manager::checkHasInternet() { + if ( d->m_hasInternetUrls.empty() ) + { + return false; + } + if ( d->m_lastCheckedUrlIndex < 0 ) + { + d->m_lastCheckedUrlIndex = 0; + } + int attempts = 0; + do + { + // Start by pinging the same one as last time + d->m_hasInternet = synchronousPing( d->m_hasInternetUrls.at( d->m_lastCheckedUrlIndex ) ); + // if it's not responding, **then** move on to the next one, + // and wrap around if needed + if ( !d->m_hasInternet ) + { + if ( ++( d->m_lastCheckedUrlIndex ) >= d->m_hasInternetUrls.size() ) + { + d->m_lastCheckedUrlIndex = 0; + } + } + // keep track of how often we've tried, because there's no point in + // going around more than once. + attempts++; + } while ( !d->m_hasInternet && ( attempts < d->m_hasInternetUrls.size() ) ); - d->m_hasInternet = synchronousPing( d->m_hasInternetUrl ); // For earlier Qt versions (< 5.15.0), set the accessibility flag to // NotAccessible if synchronous ping has failed, so that any module @@ -177,7 +203,11 @@ Manager::checkHasInternet() void Manager::setCheckHasInternetUrl( const QUrl& url ) { - d->m_hasInternetUrl = url; + if ( d->m_hasInternetUrls.empty() ) + { + d->m_lastCheckedUrlIndex = -1; + } + d->m_hasInternetUrls.append( url ); } /** @brief Does a request asynchronously, returns the (pending) reply From 81fe8b148834d236c78404e2e91539be4d689a70 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 1 Aug 2021 22:37:13 +0200 Subject: [PATCH 02/17] [libcalamares] Expand API for setting URLs to check --- src/libcalamares/network/Manager.cpp | 19 +++++++++++++++---- src/libcalamares/network/Manager.h | 7 +++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index e5277d74e..62ab60e60 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -203,10 +203,21 @@ Manager::checkHasInternet() void Manager::setCheckHasInternetUrl( const QUrl& url ) { - if ( d->m_hasInternetUrls.empty() ) - { - d->m_lastCheckedUrlIndex = -1; - } + d->m_lastCheckedUrlIndex = -1; + d->m_hasInternetUrls.clear(); + d->m_hasInternetUrls.append( url ); +} + +void +Manager::setCheckHasInternetUrl( const QVector< QUrl >& urls ) +{ + d->m_lastCheckedUrlIndex = -1; + d->m_hasInternetUrls = urls; +} + +void +Manager::addCheckHasInternetUrl( const QUrl& url ) +{ d->m_hasInternetUrls.append( url ); } diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index a038dceae..8bc3dded7 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -122,6 +123,12 @@ public: /// @brief Set the URL which is used for the general "is there internet" check. void setCheckHasInternetUrl( const QUrl& url ); + /// @brief Adds an (extra) URL to check + void addCheckHasInternetUrl( const QUrl& url ); + + /// @brief Set a collection of URLs used for the general "is there internet" check. + void setCheckHasInternetUrl( const QVector< QUrl >& urls ); + /** @brief Do a network request asynchronously. * * Returns a pointer to the reply-from-the-request. From 2f3062f4c298873700998f9304e0c2045f690e45 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 1 Aug 2021 23:49:33 +0200 Subject: [PATCH 03/17] [libcalamares] Fix typo in comment --- src/libcalamares/utils/Variant.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/utils/Variant.h b/src/libcalamares/utils/Variant.h index ab9e73f90..d0d893dc3 100644 --- a/src/libcalamares/utils/Variant.h +++ b/src/libcalamares/utils/Variant.h @@ -34,7 +34,7 @@ DLLEXPORT QString getString( const QVariantMap& map, const QString& key, const Q /** @brief Get a string list from a mapping with a given key; returns @p d if no value. * - * This is slightly more lenient that getString(), and a single-string value will + * This is slightly more lenient than getString(), and a single-string value will * be returned as a 1-item list. */ DLLEXPORT QStringList getStringList( const QVariantMap& map, const QString& key, const QStringList& d = QStringList() ); From 1452b747402928edf4ddd84431429ade859ebe82 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 1 Aug 2021 23:52:27 +0200 Subject: [PATCH 04/17] [welcome] Load potentially a list of URLs to check --- .../welcome/checker/GeneralRequirements.cpp | 76 +++++++++++++------ src/modules/welcome/welcome.conf | 15 ++++ 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/modules/welcome/checker/GeneralRequirements.cpp b/src/modules/welcome/checker/GeneralRequirements.cpp index 07c352946..2528cbd9f 100644 --- a/src/modules/welcome/checker/GeneralRequirements.cpp +++ b/src/modules/welcome/checker/GeneralRequirements.cpp @@ -215,6 +215,57 @@ GeneralRequirements::checkRequirements() return checkEntries; } +/** @brief Loads the check-internel URLs + * + * There may be zero or one or more URLs specified; returns + * @c true if the configuration is incomplete or damaged in some way. + */ +static bool +getCheckInternetUrls( const QVariantMap& configurationMap ) +{ + const QString exampleUrl = QStringLiteral( "http://example.com" ); + + bool incomplete = false; + QStringList checkInternetSetting = CalamaresUtils::getStringList( configurationMap, "internetCheckUrl" ); + if ( !checkInternetSetting.isEmpty() ) + { + QVector< QUrl > urls; + for ( const auto& urlString : qAsConst( checkInternetSetting ) ) + { + QUrl url( urlString.trimmed() ); + if ( url.isValid() ) + { + urls.append( url ); + } + else + { + cWarning() << "GeneralRequirements entry 'internetCheckUrl' in welcome.conf contains invalid" + << urlString; + } + } + + if ( urls.empty() ) + { + cWarning() << "GeneralRequirements entry 'internetCheckUrl' contains no valid URLs," + << "reverting to default (http://example.com)."; + CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( QUrl( exampleUrl ) ); + incomplete = true; + } + else + { + CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( urls ); + } + } + else + { + cWarning() << "GeneralRequirements entry 'internetCheckUrl' is undefined in welcome.conf," + "reverting to default (http://example.com)."; + CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( QUrl( exampleUrl ) ); + incomplete = true; + } + return incomplete; +} + void GeneralRequirements::setConfigurationMap( const QVariantMap& configurationMap ) @@ -302,30 +353,7 @@ GeneralRequirements::setConfigurationMap( const QVariantMap& configurationMap ) incompleteConfiguration = true; } - QUrl checkInternetUrl; - QString checkInternetSetting = CalamaresUtils::getString( configurationMap, "internetCheckUrl" ); - if ( !checkInternetSetting.isEmpty() ) - { - checkInternetUrl = QUrl( checkInternetSetting.trimmed() ); - if ( !checkInternetUrl.isValid() ) - { - cWarning() << "GeneralRequirements entry 'internetCheckUrl' is invalid in welcome.conf" - << checkInternetSetting << "reverting to default (http://example.com)."; - checkInternetUrl = QUrl( "http://example.com" ); - incompleteConfiguration = true; - } - } - else - { - cWarning() << "GeneralRequirements entry 'internetCheckUrl' is undefined in welcome.conf," - "reverting to default (http://example.com)."; - checkInternetUrl = "http://example.com"; - incompleteConfiguration = true; - } - if ( checkInternetUrl.isValid() ) - { - CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( checkInternetUrl ); - } + incompleteConfiguration |= getCheckInternetUrls( configurationMap ); if ( incompleteConfiguration ) { diff --git a/src/modules/welcome/welcome.conf b/src/modules/welcome/welcome.conf index b3da8d366..6e11817bf 100644 --- a/src/modules/welcome/welcome.conf +++ b/src/modules/welcome/welcome.conf @@ -43,6 +43,21 @@ requirements: # # The URL is only used if "internet" is in the *check* list below. internetCheckUrl: http://example.com + # + # This may be a single URL, or a list or URLs, in which case the + # URLs will be checked one-by-one; if any of them returns data, + # internet is assumed to be OK. This can be used to check via + # a number of places, where some domains may be down or blocked. + # + # To use a list of URLs, just use YAML list syntax (e.g. + # + # internetCheckUrl: + # - http://www.kde.org + # - http://www.freebsd.org + # + # or short-form + # + # internetCheckUrl: [ http://www.kde.org, http://www.freebsd.org ] # List conditions to check. Each listed condition will be # probed in some way, and yields true or false according to From c79fc2e6d91af9b39a500f3a8749182776887a43 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 10:00:42 +0200 Subject: [PATCH 05/17] [libcalamares] Add urls only if valid, add tests to check that --- src/libcalamares/network/Manager.cpp | 14 ++++++- src/libcalamares/network/Tests.cpp | 55 ++++++++++++++++++++++++++++ src/libcalamares/network/Tests.h | 3 ++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 62ab60e60..0e651f45c 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -20,6 +20,8 @@ #include #include +#include + namespace CalamaresUtils { namespace Network @@ -205,7 +207,10 @@ Manager::setCheckHasInternetUrl( const QUrl& url ) { d->m_lastCheckedUrlIndex = -1; d->m_hasInternetUrls.clear(); - d->m_hasInternetUrls.append( url ); + if ( url.isValid() ) + { + d->m_hasInternetUrls.append( url ); + } } void @@ -213,12 +218,17 @@ Manager::setCheckHasInternetUrl( const QVector< QUrl >& urls ) { d->m_lastCheckedUrlIndex = -1; d->m_hasInternetUrls = urls; + std::remove_if( + d->m_hasInternetUrls.begin(), d->m_hasInternetUrls.end(), []( const QUrl& u ) { return u.isValid(); } ); } void Manager::addCheckHasInternetUrl( const QUrl& url ) { - d->m_hasInternetUrls.append( url ); + if ( url.isValid() ) + { + d->m_hasInternetUrls.append( url ); + } } /** @brief Does a request asynchronously, returns the (pending) reply diff --git a/src/libcalamares/network/Tests.cpp b/src/libcalamares/network/Tests.cpp index d42a74115..d44f03781 100644 --- a/src/libcalamares/network/Tests.cpp +++ b/src/libcalamares/network/Tests.cpp @@ -60,3 +60,58 @@ NetworkTests::testPing() QVERIFY( canPing_www_kde_org ); } } + +void +NetworkTests::testCheckUrl() +{ + using namespace CalamaresUtils::Network; + Logger::setupLogLevel( Logger::LOGVERBOSE ); + auto& nam = Manager::instance(); + + { + QUrl u( "http://example.com" ); + QVERIFY( u.isValid() ); + nam.setCheckHasInternetUrl( u ); + QVERIFY( nam.checkHasInternet() ); + } + { + QUrl u( "http://nonexistent.example.com" ); + QVERIFY( u.isValid() ); + nam.setCheckHasInternetUrl( u ); + QVERIFY( !nam.checkHasInternet() ); + } + { + QUrl u; + QVERIFY( !u.isValid() ); + nam.setCheckHasInternetUrl( u ); + QVERIFY( !nam.checkHasInternet() ); + } +} + +void +NetworkTests::testCheckMultiUrl() +{ + using namespace CalamaresUtils::Network; + Logger::setupLogLevel( Logger::LOGVERBOSE ); + auto& nam = Manager::instance(); + + { + QUrl u0( "http://example.com" ); + QUrl u1( "https://kde.org" ); + QVERIFY( u0.isValid() ); + QVERIFY( u1.isValid() ); + nam.setCheckHasInternetUrl( { u0, u1 } ); + QVERIFY( nam.checkHasInternet() ); + } + { + QUrl u0( "http://nonexistent.example.com" ); + QUrl u1( "http://bogus.example.com" ); + QVERIFY( u0.isValid() ); + QVERIFY( u1.isValid() ); + nam.setCheckHasInternetUrl( { u0, u1 } ); + QVERIFY( !nam.checkHasInternet() ); + QVERIFY( !nam.checkHasInternet() ); + nam.addCheckHasInternetUrl( QUrl( "http://example.com" ) ); + QVERIFY( nam.checkHasInternet() ); + } +} diff --git a/src/libcalamares/network/Tests.h b/src/libcalamares/network/Tests.h index 6000e227a..d72da574a 100644 --- a/src/libcalamares/network/Tests.h +++ b/src/libcalamares/network/Tests.h @@ -24,6 +24,9 @@ private Q_SLOTS: void testInstance(); void testPing(); + + void testCheckUrl(); + void testCheckMultiUrl(); }; #endif From f1a47a9f0a16e7ba9a9e5b2e09a7906876b5e8f8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 10:24:11 +0200 Subject: [PATCH 06/17] [welcome] Add (stub) test for the Config object This crashes because there's no translations object yet, but that is an internal issue. --- src/modules/welcome/CMakeLists.txt | 10 +++ src/modules/welcome/Tests.cpp | 61 +++++++++++++++++++ .../welcome/tests/1a-checkinternet.conf | 3 + 3 files changed, 74 insertions(+) create mode 100644 src/modules/welcome/Tests.cpp create mode 100644 src/modules/welcome/tests/1a-checkinternet.conf diff --git a/src/modules/welcome/CMakeLists.txt b/src/modules/welcome/CMakeLists.txt index 924062652..4895d4b80 100644 --- a/src/modules/welcome/CMakeLists.txt +++ b/src/modules/welcome/CMakeLists.txt @@ -42,3 +42,13 @@ calamares_add_plugin( welcome Qt5::Network SHARED_LIB ) + +calamares_add_test( + welcometest + SOURCES + Config.cpp + Tests.cpp + LIBRARIES + Qt5::Widgets + Calamares::calamaresui +) diff --git a/src/modules/welcome/Tests.cpp b/src/modules/welcome/Tests.cpp new file mode 100644 index 000000000..366de416f --- /dev/null +++ b/src/modules/welcome/Tests.cpp @@ -0,0 +1,61 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2021 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +#include "Config.h" + +#include "utils/Logger.h" +#include "utils/Yaml.h" + +#include + +class WelcomeTests : public QObject +{ + Q_OBJECT +public: + WelcomeTests(); + ~WelcomeTests() override {} + +private Q_SLOTS: + void initTestCase(); + + void testOneUrl(); +}; + +WelcomeTests::WelcomeTests() {} + +void +WelcomeTests::initTestCase() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); + cDebug() << "Welcome test started."; +} + +void +WelcomeTests::testOneUrl() +{ + Config c; + + // BUILD_AS_TEST is the source-directory path + QString filename = QStringLiteral( "1a-checkinternet.conf" ); + QFile fi( QString( "%1/%2" ).arg( BUILD_AS_TEST, filename ) ); + QVERIFY( fi.exists() ); + + bool ok = false; + const auto map = CalamaresUtils::loadYaml( fi, &ok ); + QVERIFY( ok ); + QVERIFY( map.count() > 0 ); + QVERIFY( map.contains( "requirements" ) ); +} + + +QTEST_GUILESS_MAIN( WelcomeTests ) + +#include "utils/moc-warnings.h" + +#include "Tests.moc" diff --git a/src/modules/welcome/tests/1a-checkinternet.conf b/src/modules/welcome/tests/1a-checkinternet.conf new file mode 100644 index 000000000..d2140f201 --- /dev/null +++ b/src/modules/welcome/tests/1a-checkinternet.conf @@ -0,0 +1,3 @@ +--- +requirements: + internetCheckUrl: http://example.com From 38c65e80f3c61860fa7ba0b102d6012dd4ccc40a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 11:57:37 +0200 Subject: [PATCH 07/17] [libcalamaresui] Warn when asking for nonexistent Branding instance --- src/libcalamaresui/Branding.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcalamaresui/Branding.cpp b/src/libcalamaresui/Branding.cpp index b9445ba83..5894c723d 100644 --- a/src/libcalamaresui/Branding.cpp +++ b/src/libcalamaresui/Branding.cpp @@ -49,6 +49,10 @@ Branding* Branding::s_instance = nullptr; Branding* Branding::instance() { + if ( !s_instance ) + { + cWarning() << "No Branding instance created yet."; + } return s_instance; } From e9a98f35ad1b61e0d60f3c28f110fe482f99d102 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 11:58:24 +0200 Subject: [PATCH 08/17] [welcome] Avoid crash when no Branding available - don't install translators twice -- do it in setLocaleIndex only - avoid crash if the branding instance is nullptr --- src/modules/welcome/Config.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/modules/welcome/Config.cpp b/src/modules/welcome/Config.cpp index bc489d186..52cd4c88d 100644 --- a/src/modules/welcome/Config.cpp +++ b/src/modules/welcome/Config.cpp @@ -152,8 +152,6 @@ Config::initLanguages() { QString name = m_languages->locale( matchedLocaleIndex ).name(); cDebug() << Logger::SubEntry << "Matched with index" << matchedLocaleIndex << name; - - CalamaresUtils::installTranslator( name, Calamares::Branding::instance()->translationsDirectory() ); setLocaleIndex( matchedLocaleIndex ); } else @@ -192,7 +190,8 @@ Config::setLocaleIndex( int index ) cDebug() << "Index" << index << "Selected locale" << selectedLocale; QLocale::setDefault( selectedLocale ); - CalamaresUtils::installTranslator( selectedLocale, Calamares::Branding::instance()->translationsDirectory() ); + const auto* branding = Calamares::Branding::instance(); + CalamaresUtils::installTranslator( selectedLocale, branding ? branding->translationsDirectory() : QString() ); if ( Calamares::JobQueue::instance() && Calamares::JobQueue::instance()->globalStorage() ) { CalamaresUtils::Locale::insertGS( *Calamares::JobQueue::instance()->globalStorage(), From 14c26d01afb881f0114ab85e8532cf1c8fadb070 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 12:30:44 +0200 Subject: [PATCH 09/17] [libcalamares] Warnings for nullptr Settings --- src/libcalamares/Settings.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 9453075b6..2620b8563 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -104,6 +104,10 @@ Settings* Settings::s_instance = nullptr; Settings* Settings::instance() { + if ( !s_instance ) + { + cWarning() << "Getting nullptr Settings instance."; + } return s_instance; } @@ -238,6 +242,9 @@ Settings::Settings( bool debugMode ) , m_disableCancel( false ) , m_disableCancelDuringExec( false ) { + cWarning() << "Using bogus Calamares settings in" + << ( debugMode ? QStringLiteral( "debug" ) : QStringLiteral( "regular" ) ) << "mode"; + s_instance = this; } Settings::Settings( const QString& settingsFilePath, bool debugMode ) From 1e05e7996b778a6d9c7c4a5192008606ad2ea0ef Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 12:32:48 +0200 Subject: [PATCH 10/17] [libcalamares] Avoid cError + SubEntry The combination of Error and SubEntry loses the indentation. --- src/libcalamares/utils/CalamaresUtilsSystem.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 29f743743..df578a862 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -102,7 +102,7 @@ System::instance() if ( !s_instance ) { cError() << "No Calamares system-object has been created."; - cError() << Logger::SubEntry << "using a bogus instance instead."; + cDebug() << Logger::SubEntry << "using a bogus instance instead."; return new System( true, nullptr ); } return s_instance; From e0ee2d9514bba73bf384b6794cfc4f4b5a0dd000 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 12:50:03 +0200 Subject: [PATCH 11/17] [welcome] Handle nullptrs nicely in Config - Branding, Settings, and ModuleManager may all be nullptr, in which case the corresponding code shouldn't call methods of those instances -- this is demonstrated by just creating a Config object --- src/modules/welcome/Config.cpp | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/modules/welcome/Config.cpp b/src/modules/welcome/Config.cpp index 52cd4c88d..989475153 100644 --- a/src/modules/welcome/Config.cpp +++ b/src/modules/welcome/Config.cpp @@ -40,14 +40,16 @@ Config::retranslate() { cWarning() << "Retranslated to" << QLocale().name(); - m_genericWelcomeMessage = genericWelcomeMessage().arg( Calamares::Branding::instance()->versionedName() ); + const auto* branding = Calamares::Branding::instance(); + const auto* settings = Calamares::Settings::instance(); + m_genericWelcomeMessage = genericWelcomeMessage().arg( branding ? branding->versionedName() : QString() ); emit genericWelcomeMessageChanged( m_genericWelcomeMessage ); const auto* r = requirementsModel(); - if ( !r->satisfiedRequirements() ) + if ( r && !r->satisfiedRequirements() ) { QString message; - const bool setup = Calamares::Settings::instance()->isSetupMode(); + const bool setup = settings ? settings->isSetupMode() : false; if ( !r->satisfiedMandatory() ) { @@ -72,13 +74,13 @@ Config::retranslate() "might be disabled." ); } - m_warningMessage = message.arg( Calamares::Branding::instance()->shortVersionedName() ); + m_warningMessage = message.arg( branding ? branding->shortVersionedName() : QString() ); } else { m_warningMessage = tr( "This program will ask you some questions and " "set up %2 on your computer." ) - .arg( Calamares::Branding::instance()->productName() ); + .arg( branding ? branding->productName() : QString() ); } emit warningMessageChanged( m_warningMessage ); @@ -93,7 +95,8 @@ Config::languagesModel() const Calamares::RequirementsModel* Config::requirementsModel() const { - return Calamares::ModuleManager::instance()->requirementsModel(); + auto* manager = Calamares::ModuleManager::instance(); + return manager ? manager->requirementsModel() : nullptr; } QAbstractItemModel* @@ -241,17 +244,19 @@ Config::genericWelcomeMessage() const { QString message; - if ( Calamares::Settings::instance()->isSetupMode() ) + const auto* settings = Calamares::Settings::instance(); + const auto* branding = Calamares::Branding::instance(); + const bool welcomeStyle = branding ? branding->welcomeStyleCalamares() : true; + + if ( settings ? settings->isSetupMode() : false ) { - message = Calamares::Branding::instance()->welcomeStyleCalamares() - ? tr( "

Welcome to the Calamares setup program for %1

" ) - : tr( "

Welcome to %1 setup

" ); + message = welcomeStyle ? tr( "

Welcome to the Calamares setup program for %1

" ) + : tr( "

Welcome to %1 setup

" ); } else { - message = Calamares::Branding::instance()->welcomeStyleCalamares() - ? tr( "

Welcome to the Calamares installer for %1

" ) - : tr( "

Welcome to the %1 installer

" ); + message = welcomeStyle ? tr( "

Welcome to the Calamares installer for %1

" ) + : tr( "

Welcome to the %1 installer

" ); } return message; From d5e6e1075d9cdf5f32e9b7e746f31117d8986ba7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 24 Aug 2021 13:37:02 +0200 Subject: [PATCH 12/17] [welcome] Expand stub tests to check that crashes are gone --- src/modules/welcome/Tests.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/modules/welcome/Tests.cpp b/src/modules/welcome/Tests.cpp index 366de416f..eed7dae87 100644 --- a/src/modules/welcome/Tests.cpp +++ b/src/modules/welcome/Tests.cpp @@ -9,6 +9,9 @@ #include "Config.h" +#include "Branding.h" +#include "Settings.h" +#include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" #include "utils/Yaml.h" @@ -34,6 +37,17 @@ WelcomeTests::initTestCase() { Logger::setupLogLevel( Logger::LOGDEBUG ); cDebug() << "Welcome test started."; + + // Ensure we have a system object, expect it to be a "bogus" one + CalamaresUtils::System* system = CalamaresUtils::System::instance(); + QVERIFY( system ); + cDebug() << Logger::SubEntry << "System @" << Logger::Pointer( system ); + + const auto* settings = Calamares::Settings::instance(); + if ( !settings ) + { + (void)new Calamares::Settings( true ); + } } void From 653359d815ce4113ff384848e3c0dd5b430bb1e7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 26 Aug 2021 12:57:40 +0200 Subject: [PATCH 13/17] [libcalamares] Fix up multiple URLs for checkinternet - was filtering out the wrong URLs - was not actually removing the invalid URLs - extend API to make it possible to count / confirm the settings - extend tests to demonstrate that API and the issues --- src/libcalamares/network/Manager.cpp | 14 ++++++++++++-- src/libcalamares/network/Manager.h | 4 ++++ src/libcalamares/network/Tests.cpp | 20 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 0e651f45c..5125f592e 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -218,8 +218,12 @@ Manager::setCheckHasInternetUrl( const QVector< QUrl >& urls ) { d->m_lastCheckedUrlIndex = -1; d->m_hasInternetUrls = urls; - std::remove_if( - d->m_hasInternetUrls.begin(), d->m_hasInternetUrls.end(), []( const QUrl& u ) { return u.isValid(); } ); + auto it = std::remove_if( + d->m_hasInternetUrls.begin(), d->m_hasInternetUrls.end(), []( const QUrl& u ) { return !u.isValid(); } ); + if ( it != d->m_hasInternetUrls.end() ) + { + d->m_hasInternetUrls.erase( it ); + } } void @@ -231,6 +235,12 @@ Manager::addCheckHasInternetUrl( const QUrl& url ) } } +QVector< QUrl > +Manager::getCheckInternetUrls() const +{ + return d->m_hasInternetUrls; +} + /** @brief Does a request asynchronously, returns the (pending) reply * * The extra options for the request are taken from @p options, diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index 8bc3dded7..6a906c883 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -90,6 +90,7 @@ class DLLEXPORT Manager : public QObject { Q_OBJECT Q_PROPERTY( bool hasInternet READ hasInternet NOTIFY hasInternetChanged FINAL ) + Q_PROPERTY( QVector< QUrl > checkInternetUrls READ getCheckInternetUrls WRITE setCheckHasInternetUrl ) Manager(); @@ -129,6 +130,9 @@ public: /// @brief Set a collection of URLs used for the general "is there internet" check. void setCheckHasInternetUrl( const QVector< QUrl >& urls ); + /// @brief What URLs are used to check for internet connectivity? + QVector< QUrl > getCheckInternetUrls() const; + /** @brief Do a network request asynchronously. * * Returns a pointer to the reply-from-the-request. diff --git a/src/libcalamares/network/Tests.cpp b/src/libcalamares/network/Tests.cpp index d44f03781..a5bc52497 100644 --- a/src/libcalamares/network/Tests.cpp +++ b/src/libcalamares/network/Tests.cpp @@ -30,6 +30,7 @@ NetworkTests::testInstance() { auto& nam = CalamaresUtils::Network::Manager::instance(); QVERIFY( !nam.hasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 0 ); } void @@ -73,18 +74,21 @@ NetworkTests::testCheckUrl() QVERIFY( u.isValid() ); nam.setCheckHasInternetUrl( u ); QVERIFY( nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 1 ); // Valid URL } { QUrl u( "http://nonexistent.example.com" ); QVERIFY( u.isValid() ); nam.setCheckHasInternetUrl( u ); QVERIFY( !nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 1 ); // Valid URL even if it doesn't resolve } { QUrl u; QVERIFY( !u.isValid() ); nam.setCheckHasInternetUrl( u ); QVERIFY( !nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 0 ); // Invalid URL tried } } @@ -102,6 +106,7 @@ NetworkTests::testCheckMultiUrl() QVERIFY( u1.isValid() ); nam.setCheckHasInternetUrl( { u0, u1 } ); QVERIFY( nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 2 ); } { QUrl u0( "http://nonexistent.example.com" ); @@ -111,7 +116,22 @@ NetworkTests::testCheckMultiUrl() nam.setCheckHasInternetUrl( { u0, u1 } ); QVERIFY( !nam.checkHasInternet() ); QVERIFY( !nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 2 ); // Both are valid URLs nam.addCheckHasInternetUrl( QUrl( "http://example.com" ) ); QVERIFY( nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 3 ); + } + { + QUrl u0( "http://nonexistent.example.com" ); + QUrl u1; + QVERIFY( u0.isValid() ); + QVERIFY( !u1.isValid() ); + nam.setCheckHasInternetUrl( { u0, u1 } ); + QVERIFY( !nam.checkHasInternet() ); + QVERIFY( !nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 1 ); // Only valid URL added + nam.addCheckHasInternetUrl( QUrl( "http://example.com" ) ); + QVERIFY( nam.checkHasInternet() ); + QCOMPARE( nam.getCheckInternetUrls().count(), 2 ); } } From 05388814476f6e48bb70bcb82aeef6da7cb46ab7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 26 Aug 2021 14:32:57 +0200 Subject: [PATCH 14/17] [libcalamares] Handle multiple invalid URLs at once - expand tests with example where more than one URL is invalid - fix the call to the wrong overload of QVector::erase() --- src/libcalamares/network/Manager.cpp | 2 +- src/libcalamares/network/Tests.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 5125f592e..cceff477e 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -222,7 +222,7 @@ Manager::setCheckHasInternetUrl( const QVector< QUrl >& urls ) d->m_hasInternetUrls.begin(), d->m_hasInternetUrls.end(), []( const QUrl& u ) { return !u.isValid(); } ); if ( it != d->m_hasInternetUrls.end() ) { - d->m_hasInternetUrls.erase( it ); + d->m_hasInternetUrls.erase( it, d->m_hasInternetUrls.end() ); } } diff --git a/src/libcalamares/network/Tests.cpp b/src/libcalamares/network/Tests.cpp index a5bc52497..e5bd34c23 100644 --- a/src/libcalamares/network/Tests.cpp +++ b/src/libcalamares/network/Tests.cpp @@ -134,4 +134,14 @@ NetworkTests::testCheckMultiUrl() QVERIFY( nam.checkHasInternet() ); QCOMPARE( nam.getCheckInternetUrls().count(), 2 ); } + { + QUrl u0( "http://nonexistent.example.com" ); + QUrl u1; + QVERIFY( u0.isValid() ); + QVERIFY( !u1.isValid() ); + nam.setCheckHasInternetUrl( { u1, u1, u1, u1 } ); + QCOMPARE( nam.getCheckInternetUrls().count(), 0 ); + nam.setCheckHasInternetUrl( { u1, u1, u0, u1 } ); + QCOMPARE( nam.getCheckInternetUrls().count(), 1 ); + } } From 67d2b5568d54a8cb2bcc42efb96a761987214561 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 26 Aug 2021 14:40:52 +0200 Subject: [PATCH 15/17] [welcome] Fix test, check that the internet check URLs are loaded --- src/modules/welcome/Tests.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/modules/welcome/Tests.cpp b/src/modules/welcome/Tests.cpp index eed7dae87..cc2b7b873 100644 --- a/src/modules/welcome/Tests.cpp +++ b/src/modules/welcome/Tests.cpp @@ -11,6 +11,7 @@ #include "Branding.h" #include "Settings.h" +#include "network/Manager.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" #include "utils/Yaml.h" @@ -57,7 +58,7 @@ WelcomeTests::testOneUrl() // BUILD_AS_TEST is the source-directory path QString filename = QStringLiteral( "1a-checkinternet.conf" ); - QFile fi( QString( "%1/%2" ).arg( BUILD_AS_TEST, filename ) ); + QFile fi( QString( "%1/tests/%2" ).arg( BUILD_AS_TEST, filename ) ); QVERIFY( fi.exists() ); bool ok = false; @@ -65,6 +66,9 @@ WelcomeTests::testOneUrl() QVERIFY( ok ); QVERIFY( map.count() > 0 ); QVERIFY( map.contains( "requirements" ) ); + + c.setConfigurationMap( map ); + QCOMPARE( CalamaresUtils::Network::Manager::instance().getCheckInternetUrls().count(), 1 ); } From ad76a2cbe870ca9d67f050654e6add3dbfbe2fd5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 26 Aug 2021 16:39:55 +0200 Subject: [PATCH 16/17] [welcome] [welcomeq] Move requirements to Config The Config object can hold all of the configuration information, including also the requirements-checking parts. Move requirements- checking configuration there, so it is shared and consistent across welcome and welcomeq, regardless. This repairs the test that expects the Config object to handle **all** of the configuration, too. --- src/modules/welcome/CMakeLists.txt | 25 +++++++++++---------- src/modules/welcome/Config.cpp | 12 ++++++++++ src/modules/welcome/Config.h | 6 +++++ src/modules/welcome/WelcomeViewStep.cpp | 15 +------------ src/modules/welcome/WelcomeViewStep.h | 1 - src/modules/welcomeq/WelcomeQmlViewStep.cpp | 18 +-------------- src/modules/welcomeq/WelcomeQmlViewStep.h | 3 --- 7 files changed, 33 insertions(+), 47 deletions(-) diff --git a/src/modules/welcome/CMakeLists.txt b/src/modules/welcome/CMakeLists.txt index 4895d4b80..01a89703a 100644 --- a/src/modules/welcome/CMakeLists.txt +++ b/src/modules/welcome/CMakeLists.txt @@ -8,26 +8,22 @@ find_package( Qt5 ${QT_VERSION} CONFIG REQUIRED DBus Network ) find_package( LIBPARTED ) if ( LIBPARTED_FOUND ) set( PARTMAN_SRC checker/partman_devices.c ) - set( CHECKER_LINK_LIBRARIES ${LIBPARTED_LIBRARY} ) + set( PARTMAN_LIB ${LIBPARTED_LIBRARY} ) else() set( PARTMAN_SRC ) - set( CHECKER_LINK_LIBRARIES ) + set( PARTMAN_LIB ) add_definitions( -DWITHOUT_LIBPARTED ) endif() -set( CHECKER_SOURCES - checker/CheckerContainer.cpp - checker/GeneralRequirements.cpp - checker/ResultWidget.cpp - checker/ResultsListWidget.cpp - ${PARTMAN_SRC} -) - calamares_add_plugin( welcome TYPE viewmodule EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES - ${CHECKER_SOURCES} + checker/CheckerContainer.cpp + checker/GeneralRequirements.cpp + checker/ResultWidget.cpp + checker/ResultsListWidget.cpp + ${PARTMAN_SRC} WelcomeViewStep.cpp Config.cpp Config.h @@ -37,7 +33,7 @@ calamares_add_plugin( welcome RESOURCES welcome.qrc LINK_PRIVATE_LIBRARIES - ${CHECKER_LINK_LIBRARIES} + ${PARTMAN_LIB} Qt5::DBus Qt5::Network SHARED_LIB @@ -46,9 +42,14 @@ calamares_add_plugin( welcome calamares_add_test( welcometest SOURCES + checker/GeneralRequirements.cpp + ${PARTMAN_SRC} Config.cpp Tests.cpp LIBRARIES + ${PARTMAN_LIB} + Qt5::DBus + Qt5::Network Qt5::Widgets Calamares::calamaresui ) diff --git a/src/modules/welcome/Config.cpp b/src/modules/welcome/Config.cpp index 989475153..c6b960d9d 100644 --- a/src/modules/welcome/Config.cpp +++ b/src/modules/welcome/Config.cpp @@ -27,6 +27,7 @@ Config::Config( QObject* parent ) : QObject( parent ) , m_languages( CalamaresUtils::Locale::availableTranslations() ) , m_filtermodel( std::make_unique< QSortFilterProxyModel >() ) + , m_requirementsChecker( std::make_unique< GeneralRequirements >( this ) ) { initLanguages(); @@ -399,4 +400,15 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) ::setLanguageIcon( this, configurationMap ); ::setGeoIP( this, configurationMap ); + + if ( configurationMap.contains( "requirements" ) + && configurationMap.value( "requirements" ).type() == QVariant::Map ) + { + m_requirementsChecker->setConfigurationMap( configurationMap.value( "requirements" ).toMap() ); + } + else + { + cWarning() << "no valid requirements map found in welcome " + "module configuration."; + } } diff --git a/src/modules/welcome/Config.h b/src/modules/welcome/Config.h index a3f1276a6..6cce75f22 100644 --- a/src/modules/welcome/Config.h +++ b/src/modules/welcome/Config.h @@ -10,6 +10,8 @@ #ifndef WELCOME_CONFIG_H #define WELCOME_CONFIG_H +#include "checker/GeneralRequirements.h" + #include "locale/LabelModel.h" #include "modulesystem/RequirementsModel.h" @@ -100,6 +102,9 @@ public slots: QAbstractItemModel* unsatisfiedRequirements() const; + /// @brief Check the general requirements + Calamares::RequirementsList checkRequirements() const { return m_requirementsChecker->checkRequirements(); } + signals: void countryCodeChanged( QString countryCode ); void localeIndexChanged( int localeIndex ); @@ -118,6 +123,7 @@ private: CalamaresUtils::Locale::LabelModel* m_languages = nullptr; std::unique_ptr< QSortFilterProxyModel > m_filtermodel; + std::unique_ptr< GeneralRequirements > m_requirementsChecker; QString m_languageIcon; QString m_countryCode; diff --git a/src/modules/welcome/WelcomeViewStep.cpp b/src/modules/welcome/WelcomeViewStep.cpp index 2a0d57bc4..df42271fc 100644 --- a/src/modules/welcome/WelcomeViewStep.cpp +++ b/src/modules/welcome/WelcomeViewStep.cpp @@ -12,7 +12,6 @@ #include "Config.h" #include "WelcomePage.h" -#include "checker/GeneralRequirements.h" #include "Branding.h" #include "modulesystem/ModuleManager.h" @@ -25,7 +24,6 @@ WelcomeViewStep::WelcomeViewStep( QObject* parent ) : Calamares::ViewStep( parent ) , m_conf( new Config( this ) ) , m_widget( new WelcomePage( m_conf ) ) - , m_requirementsChecker( new GeneralRequirements( this ) ) { connect( Calamares::ModuleManager::instance(), &Calamares::ModuleManager::requirementsComplete, @@ -96,17 +94,6 @@ WelcomeViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { m_conf->setConfigurationMap( configurationMap ); - if ( configurationMap.contains( "requirements" ) - && configurationMap.value( "requirements" ).type() == QVariant::Map ) - { - m_requirementsChecker->setConfigurationMap( configurationMap.value( "requirements" ).toMap() ); - } - else - { - cWarning() << "no valid requirements map found in welcome " - "module configuration."; - } - //here init the qml or qwidgets needed bits m_widget->init(); } @@ -114,5 +101,5 @@ WelcomeViewStep::setConfigurationMap( const QVariantMap& configurationMap ) Calamares::RequirementsList WelcomeViewStep::checkRequirements() { - return m_requirementsChecker->checkRequirements(); + return m_conf->checkRequirements(); } diff --git a/src/modules/welcome/WelcomeViewStep.h b/src/modules/welcome/WelcomeViewStep.h index 57632f7ac..dfc6f1169 100644 --- a/src/modules/welcome/WelcomeViewStep.h +++ b/src/modules/welcome/WelcomeViewStep.h @@ -66,7 +66,6 @@ public: private: Config* m_conf; WelcomePage* m_widget; - GeneralRequirements* m_requirementsChecker; }; CALAMARES_PLUGIN_FACTORY_DECLARATION( WelcomeViewStepFactory ) diff --git a/src/modules/welcomeq/WelcomeQmlViewStep.cpp b/src/modules/welcomeq/WelcomeQmlViewStep.cpp index af32f2992..914ea4f3a 100644 --- a/src/modules/welcomeq/WelcomeQmlViewStep.cpp +++ b/src/modules/welcomeq/WelcomeQmlViewStep.cpp @@ -26,7 +26,6 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( WelcomeQmlViewStepFactory, registerPlugin< WelcomeQmlViewStep::WelcomeQmlViewStep( QObject* parent ) : Calamares::QmlViewStep( parent ) , m_config( new Config( this ) ) - , m_requirementsChecker( new GeneralRequirements( this ) ) { connect( Calamares::ModuleManager::instance(), &Calamares::ModuleManager::requirementsComplete, @@ -58,7 +57,6 @@ WelcomeQmlViewStep::isBackEnabled() const bool WelcomeQmlViewStep::isAtBeginning() const { - // TODO: adjust to "pages" in the QML return true; } @@ -66,7 +64,6 @@ WelcomeQmlViewStep::isAtBeginning() const bool WelcomeQmlViewStep::isAtEnd() const { - // TODO: adjust to "pages" in the QML return true; } @@ -81,26 +78,13 @@ void WelcomeQmlViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { m_config->setConfigurationMap( configurationMap ); - - if ( configurationMap.contains( "requirements" ) - && configurationMap.value( "requirements" ).type() == QVariant::Map ) - { - m_requirementsChecker->setConfigurationMap( configurationMap.value( "requirements" ).toMap() ); - } - else - { - cWarning() << "no valid requirements map found in welcomeq " - "module configuration."; - } - Calamares::QmlViewStep::setConfigurationMap( configurationMap ); // call parent implementation last - setContextProperty( "Welcome", m_config ); } Calamares::RequirementsList WelcomeQmlViewStep::checkRequirements() { - return m_requirementsChecker->checkRequirements(); + return m_config->checkRequirements(); } QObject* diff --git a/src/modules/welcomeq/WelcomeQmlViewStep.h b/src/modules/welcomeq/WelcomeQmlViewStep.h index 4c68ea2ae..1ed90ce05 100644 --- a/src/modules/welcomeq/WelcomeQmlViewStep.h +++ b/src/modules/welcomeq/WelcomeQmlViewStep.h @@ -29,9 +29,7 @@ class Handler; } } // namespace CalamaresUtils -class GeneralRequirements; -// TODO: Needs a generic Calamares::QmlViewStep as base class // TODO: refactor and move what makes sense to base class class PLUGINDLLEXPORT WelcomeQmlViewStep : public Calamares::QmlViewStep { @@ -65,7 +63,6 @@ public: private: Config* m_config; - GeneralRequirements* m_requirementsChecker; }; CALAMARES_PLUGIN_FACTORY_DECLARATION( WelcomeQmlViewStepFactory ) From f376b42c31e4befd9d87c55ac80130bfd165f492 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 26 Aug 2021 16:58:46 +0200 Subject: [PATCH 17/17] [welcome] Add a handful of tests for different URL configs --- src/modules/welcome/Tests.cpp | 37 +++++++++++++++++++ .../welcome/tests/1b-checkinternet.conf | 3 ++ .../welcome/tests/1c-checkinternet.conf | 4 ++ .../welcome/tests/1d-checkinternet.conf | 4 ++ .../welcome/tests/1e-checkinternet.conf | 4 ++ .../welcome/tests/1f-checkinternet.conf | 6 +++ .../welcome/tests/1g-checkinternet.conf | 3 ++ .../welcome/tests/1h-checkinternet.conf | 8 ++++ 8 files changed, 69 insertions(+) create mode 100644 src/modules/welcome/tests/1b-checkinternet.conf create mode 100644 src/modules/welcome/tests/1c-checkinternet.conf create mode 100644 src/modules/welcome/tests/1d-checkinternet.conf create mode 100644 src/modules/welcome/tests/1e-checkinternet.conf create mode 100644 src/modules/welcome/tests/1f-checkinternet.conf create mode 100644 src/modules/welcome/tests/1g-checkinternet.conf create mode 100644 src/modules/welcome/tests/1h-checkinternet.conf diff --git a/src/modules/welcome/Tests.cpp b/src/modules/welcome/Tests.cpp index cc2b7b873..0445433e9 100644 --- a/src/modules/welcome/Tests.cpp +++ b/src/modules/welcome/Tests.cpp @@ -29,6 +29,8 @@ private Q_SLOTS: void initTestCase(); void testOneUrl(); + void testUrls_data(); + void testUrls(); }; WelcomeTests::WelcomeTests() {} @@ -71,6 +73,41 @@ WelcomeTests::testOneUrl() QCOMPARE( CalamaresUtils::Network::Manager::instance().getCheckInternetUrls().count(), 1 ); } +void +WelcomeTests::testUrls_data() +{ + QTest::addColumn< QString >( "filename" ); + QTest::addColumn< int >( "result" ); + + QTest::newRow( "one " ) << QString( "1a-checkinternet.conf" ) << 1; + QTest::newRow( "none " ) << QString( "1b-checkinternet.conf" ) << 0; + QTest::newRow( "blank" ) << QString( "1c-checkinternet.conf" ) << 0; + QTest::newRow( "bogus" ) << QString( "1d-checkinternet.conf" ) << 0; + QTest::newRow( "[] " ) << QString( "1e-checkinternet.conf" ) << 0; + QTest::newRow( "-3 " ) << QString( "1f-checkinternet.conf" ) << 3; + QTest::newRow( "[3] " ) << QString( "1g-checkinternet.conf" ) << 3; + QTest::newRow( "some " ) << QString( "1h-checkinternet.conf" ) << 3; +} + +void +WelcomeTests::testUrls() +{ + QFETCH( QString, filename ); + QFETCH( int, result ); + + Config c; + + // BUILD_AS_TEST is the source-directory path + QFile fi( QString( "%1/tests/%2" ).arg( BUILD_AS_TEST, filename ) ); + QVERIFY( fi.exists() ); + + bool ok = false; + const auto map = CalamaresUtils::loadYaml( fi, &ok ); + QVERIFY( ok ); + + c.setConfigurationMap( map ); + QCOMPARE( CalamaresUtils::Network::Manager::instance().getCheckInternetUrls().count(), result ); +} QTEST_GUILESS_MAIN( WelcomeTests ) diff --git a/src/modules/welcome/tests/1b-checkinternet.conf b/src/modules/welcome/tests/1b-checkinternet.conf new file mode 100644 index 000000000..a1e656985 --- /dev/null +++ b/src/modules/welcome/tests/1b-checkinternet.conf @@ -0,0 +1,3 @@ +# Nothing at all +--- +bogus: 1 diff --git a/src/modules/welcome/tests/1c-checkinternet.conf b/src/modules/welcome/tests/1c-checkinternet.conf new file mode 100644 index 000000000..845e253c0 --- /dev/null +++ b/src/modules/welcome/tests/1c-checkinternet.conf @@ -0,0 +1,4 @@ +# Set to blank +--- +requirements: + internetCheckUrl: "" diff --git a/src/modules/welcome/tests/1d-checkinternet.conf b/src/modules/welcome/tests/1d-checkinternet.conf new file mode 100644 index 000000000..9a44d7c93 --- /dev/null +++ b/src/modules/welcome/tests/1d-checkinternet.conf @@ -0,0 +1,4 @@ +# Set to something broken +--- +requirements: + internetCheckUrl: false diff --git a/src/modules/welcome/tests/1e-checkinternet.conf b/src/modules/welcome/tests/1e-checkinternet.conf new file mode 100644 index 000000000..579414a79 --- /dev/null +++ b/src/modules/welcome/tests/1e-checkinternet.conf @@ -0,0 +1,4 @@ +# Empty list +--- +requirements: + internetCheckUrl: [] diff --git a/src/modules/welcome/tests/1f-checkinternet.conf b/src/modules/welcome/tests/1f-checkinternet.conf new file mode 100644 index 000000000..660760381 --- /dev/null +++ b/src/modules/welcome/tests/1f-checkinternet.conf @@ -0,0 +1,6 @@ +--- +requirements: + internetCheckUrl: + - http://example.com + - http://bogus.example.com + - http://nonexistent.example.com diff --git a/src/modules/welcome/tests/1g-checkinternet.conf b/src/modules/welcome/tests/1g-checkinternet.conf new file mode 100644 index 000000000..dd3ddae0c --- /dev/null +++ b/src/modules/welcome/tests/1g-checkinternet.conf @@ -0,0 +1,3 @@ +--- +requirements: + internetCheckUrl: [ http://example.com, http://bogus.example.com, http://nonexistent.example.com ] diff --git a/src/modules/welcome/tests/1h-checkinternet.conf b/src/modules/welcome/tests/1h-checkinternet.conf new file mode 100644 index 000000000..928360c20 --- /dev/null +++ b/src/modules/welcome/tests/1h-checkinternet.conf @@ -0,0 +1,8 @@ +# "0" is a valid URL (?) but "" is not +--- +requirements: + internetCheckUrl: + - http://example.com + - 0 + - "" + - http://nonexistent.example.com