[welcome] Fix up and expand tests

- improve logging
- fix failing tests -- the observed and expected behavior is
  to fill in a fallback check-URL, not change to an empty list,
- **except** if there's no requirements key in the config
  at all; this is a bit weird, but let's make the tests
  document existing behavior so we can notice if it changes.

src/modules/welcome/Tests.cpp

@@ -31,6 +31,8 @@ private Q_SLOTS:
     void testOneUrl();
     void testUrls_data();
     void testUrls();
+
+    void testBadConfigDoesNotResetUrls();
 };
 
 WelcomeTests::WelcomeTests() {}
@@ -81,9 +83,9 @@ WelcomeTests::testUrls_data()
 
     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( "blank" ) << QString( "1c-checkinternet.conf" ) << 1;
-    QTest::newRow( "bogus" ) << QString( "1d-checkinternet.conf" ) << 0;
+    QTest::newRow( "bogus" ) << QString( "1d-checkinternet.conf" ) << 1;
-    QTest::newRow( "[]   " ) << QString( "1e-checkinternet.conf" ) << 0;
+    QTest::newRow( "[]   " ) << QString( "1e-checkinternet.conf" ) << 1;
     QTest::newRow( "-3   " ) << QString( "1f-checkinternet.conf" ) << 3;
     QTest::newRow( "[3]  " ) << QString( "1g-checkinternet.conf" ) << 3;
     QTest::newRow( "some " ) << QString( "1h-checkinternet.conf" ) << 3;
@@ -105,10 +107,59 @@ WelcomeTests::testUrls()
     const auto map = CalamaresUtils::loadYaml( fi, &ok );
     QVERIFY( ok );
 
+    CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( QVector< QUrl > {} );
+    QCOMPARE( CalamaresUtils::Network::Manager::instance().getCheckInternetUrls().count(), 0 );
     c.setConfigurationMap( map );
     QCOMPARE( CalamaresUtils::Network::Manager::instance().getCheckInternetUrls().count(), result );
 }
 
+void
+WelcomeTests::testBadConfigDoesNotResetUrls()
+{
+    auto& nam = CalamaresUtils::Network::Manager::instance();
+    CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( QVector< QUrl > {} );
+    QCOMPARE( nam.getCheckInternetUrls().count(), 0 );
+    nam.setCheckHasInternetUrl( QVector< QUrl > { QUrl( "http://example.com" ), QUrl( "https://www.kde.org" ) } );
+    QCOMPARE( nam.getCheckInternetUrls().count(), 2 );
+
+    Config c;
+
+    // This is slightly surprising: if there is **no** requirements
+    // configuration, the list of check-URLs is left unchanged.
+    {
+        const QString filename = QStringLiteral( "1b-checkinternet.conf" );  // "none"
+
+        // 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( nam.getCheckInternetUrls().count(), 2 );
+
+    // But if the config contains a requirements entry, even if broken,
+    // the list is changed (to the default).
+    {
+        const QString filename = QStringLiteral( "1d-checkinternet.conf" );  // "bogus"
+
+        // 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( nam.getCheckInternetUrls().count(), 1 );
+}
+
+
 QTEST_GUILESS_MAIN( WelcomeTests )
 
 #include "utils/moc-warnings.h"

src/modules/welcome/checker/GeneralRequirements.cpp

@@ -215,7 +215,7 @@ GeneralRequirements::checkRequirements()
     return checkEntries;
 }
 
-/** @brief Loads the check-internel URLs
+/** @brief Loads the check-internet URLs
  *
  * There may be zero or one or more URLs specified; returns
  * @c true if the configuration is incomplete or damaged in some way.
@@ -246,8 +246,8 @@ getCheckInternetUrls( const QVariantMap& configurationMap )
 
         if ( urls.empty() )
         {
-            cWarning() << "GeneralRequirements entry 'internetCheckUrl' contains no valid URLs,"
+            cWarning() << "GeneralRequirements entry 'internetCheckUrl' contains no valid URLs, "
-                       << "reverting to default (http://example.com).";
+                       << "reverting to default (" << exampleUrl << ").";
             CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( QUrl( exampleUrl ) );
             incomplete = true;
         }
@@ -258,8 +258,9 @@ getCheckInternetUrls( const QVariantMap& configurationMap )
     }
     else
     {
-        cWarning() << "GeneralRequirements entry 'internetCheckUrl' is undefined in welcome.conf,"
+        cWarning() << "GeneralRequirements entry 'internetCheckUrl' is undefined in welcome.conf, "
-                      "reverting to default (http://example.com).";
+                      "reverting to default ("
+                   << exampleUrl << ").";
         CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( QUrl( exampleUrl ) );
         incomplete = true;
     }