From 3c398bd15eca0dc33e6f8a29365ea0dc10f3b12e Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 13 Apr 2021 16:45:01 +0200
Subject: [PATCH 01/13] [netinstall] Only wrap-up if the packages list is OK

Avoid situation where the YAML is ok but doesn't contain
a list of netinstall packages, so the packages list (the model)
is still empty.

FIXES #1673
---
 src/modules/netinstall/Config.h        |  4 +++-
 src/modules/netinstall/LoaderQueue.cpp | 19 ++++++++++++++-----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index b676a7d39..58931c636 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -49,10 +49,12 @@ public:
         FailedNetworkError,
         FailedBadData,
         FailedNoData
-
     };
 
+    /// Human-readable, translated representation of the status
     QString status() const;
+    /// Internal code for the status
+    Status statusCode() const { return m_status; }
     void setStatus( Status s );
 
     bool required() const { return m_required; }
diff --git a/src/modules/netinstall/LoaderQueue.cpp b/src/modules/netinstall/LoaderQueue.cpp
index f8ba17cff..76307d380 100644
--- a/src/modules/netinstall/LoaderQueue.cpp
+++ b/src/modules/netinstall/LoaderQueue.cpp
@@ -25,6 +25,9 @@
  * On destruction, a new call to fetchNext() is queued, so that
  * the queue continues loading. Calling release() before the
  * destructor skips the fetchNext(), ending the queue-loading.
+ *
+ * Calling done(b) is the same as release(), **plus** done()
+ * is called on the queue if @p b is @c true.
  */
 class FetchNextUnless
 {
@@ -41,6 +44,14 @@ public:
         }
     }
     void release() { m_q = nullptr; }
+    void done( bool b )
+    {
+        if ( b && m_q )
+        {
+            QMetaObject::invokeMethod( m_q, "done", Qt::QueuedConnection );
+        }
+        release();
+    }
 
 private:
     LoaderQueue* m_q = nullptr;
@@ -138,7 +149,7 @@ LoaderQueue::fetch( const QUrl& url )
 void
 LoaderQueue::dataArrived()
 {
-    FetchNextUnless finished( this );
+    FetchNextUnless next( this );
 
     if ( !m_reply || !m_reply->isFinished() )
     {
@@ -170,16 +181,14 @@ LoaderQueue::dataArrived()
 
         if ( groups.IsSequence() )
         {
-            finished.release();
             m_config->loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
-            emit done();
+            next.done( m_config->statusCode() == Config::Status::Ok );
         }
         else if ( groups.IsMap() )
         {
-            finished.release();
             auto map = CalamaresUtils::yamlMapToVariant( groups );
             m_config->loadGroupList( map.value( "groups" ).toList() );
-            emit done();
+            next.done( m_config->statusCode() == Config::Status::Ok );
         }
         else
         {

From 5af37b0be3e3e922f1f5fa8697fb713748ebb334 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 13 Apr 2021 20:17:28 +0200
Subject: [PATCH 02/13] [netinstall] Stub of tests for fallback-loading

---
 src/modules/netinstall/CMakeLists.txt         |  2 +
 src/modules/netinstall/Tests.cpp              | 69 +++++++++++++++++++
 .../netinstall/tests/1a-single-bad.conf       |  7 ++
 .../netinstall/tests/1b-single-small.conf     |  7 ++
 src/modules/netinstall/tests/data-small.yaml  | 12 ++++
 5 files changed, 97 insertions(+)
 create mode 100644 src/modules/netinstall/tests/1a-single-bad.conf
 create mode 100644 src/modules/netinstall/tests/1b-single-small.conf
 create mode 100644 src/modules/netinstall/tests/data-small.yaml

diff --git a/src/modules/netinstall/CMakeLists.txt b/src/modules/netinstall/CMakeLists.txt
index ec926c9d3..6b7270db1 100644
--- a/src/modules/netinstall/CMakeLists.txt
+++ b/src/modules/netinstall/CMakeLists.txt
@@ -26,6 +26,8 @@ calamares_add_test(
     netinstalltest
     SOURCES
         Tests.cpp
+        Config.cpp
+        LoaderQueue.cpp
         PackageTreeItem.cpp
         PackageModel.cpp
     LIBRARIES
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index 0b59658c1..b8826c6be 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -7,13 +7,17 @@
  *
  */
 
+#include "Config.h"
 #include "PackageModel.h"
 #include "PackageTreeItem.h"
 
 #include "utils/Logger.h"
+#include "utils/NamedEnum.h"
 #include "utils/Variant.h"
 #include "utils/Yaml.h"
 
+#include <KMacroExpander>
+
 #include <QtTest/QtTest>
 
 class ItemTests : public QObject
@@ -40,6 +44,9 @@ private Q_SLOTS:
     void testCompare();
     void testModel();
     void testExampleFiles();
+
+    void testUrlFallback_data();
+    void testUrlFallback();
 };
 
 ItemTests::ItemTests() {}
@@ -326,6 +333,68 @@ ItemTests::testExampleFiles()
     }
 }
 
+void
+ItemTests::testUrlFallback_data()
+{
+    QTest::addColumn< QString >( "filename" );
+    QTest::addColumn< int >( "status" );
+    QTest::addColumn< int >( "count" );
+
+    using S = Config::Status;
+
+    QTest::newRow( "first" ) << "tests/1a-single-bad.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "second" ) << "tests/1b-single-small.conf" << smash( S::Ok ) << 2;
+}
+
+void
+ItemTests::testUrlFallback()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+    QFETCH( QString, filename );
+    QFETCH( int, status );
+    QFETCH( int, count );
+
+    cDebug() << "Loading" << filename;
+
+    // BUILD_AS_TEST is the source-directory path
+    QFile fi( QString( "%1/%2" ).arg( BUILD_AS_TEST, filename ) );
+    QVERIFY( fi.exists() );
+
+    Config c;
+
+    QFile yamlFile( fi.fileName() );
+    if ( yamlFile.exists() && yamlFile.open( QFile::ReadOnly | QFile::Text ) )
+    {
+        QString ba( yamlFile.readAll() );
+        QVERIFY( ba.length() > 0 );
+        QHash< QString, QString > replace;
+        replace.insert( "TESTDIR", BUILD_AS_TEST );
+        QString correctedDocument = KMacroExpander::expandMacros( ba, replace, '$' );
+
+        try
+        {
+            YAML::Node yamldoc = YAML::Load( correctedDocument.toUtf8() );
+            auto map = CalamaresUtils::yamlToVariant( yamldoc ).toMap();
+            QVERIFY( map.count() > 0 );
+            c.setConfigurationMap( map );
+        }
+        catch ( YAML::Exception& e )
+        {
+            bool badYaml = true;
+            QVERIFY( !badYaml );
+        }
+    }
+    else
+    {
+        QCOMPARE( QStringLiteral( "not found" ), fi.fileName() );
+    }
+
+    // Each of the configs sets required to **true**, which is not the default
+    QVERIFY( c.required() );
+    QCOMPARE( smash( c.statusCode() ), status );
+    QCOMPARE( c.model()->rowCount(), count );
+}
+
 
 QTEST_GUILESS_MAIN( ItemTests )
 
diff --git a/src/modules/netinstall/tests/1a-single-bad.conf b/src/modules/netinstall/tests/1a-single-bad.conf
new file mode 100644
index 000000000..c08d3870c
--- /dev/null
+++ b/src/modules/netinstall/tests/1a-single-bad.conf
@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/bad.yaml
diff --git a/src/modules/netinstall/tests/1b-single-small.conf b/src/modules/netinstall/tests/1b-single-small.conf
new file mode 100644
index 000000000..2de9b4db2
--- /dev/null
+++ b/src/modules/netinstall/tests/1b-single-small.conf
@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-small.yaml
diff --git a/src/modules/netinstall/tests/data-small.yaml b/src/modules/netinstall/tests/data-small.yaml
new file mode 100644
index 000000000..8e92736b2
--- /dev/null
+++ b/src/modules/netinstall/tests/data-small.yaml
@@ -0,0 +1,12 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+- name: "Default"
+  description: "Default group"
+  hidden: true
+  selected: true
+  critical: false
+  packages:
+    - base
+    - chakra-live-skel

From 294d07db7b8d517c604add592df81c4ac8ef33ec Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Wed, 14 Apr 2021 11:32:04 +0200
Subject: [PATCH 03/13] [netinstall] When starting to load YAML data, set
 appropriate status

- if a list is required, then we don't have data yet and should complain;
  otherwise we're OK even if no data is ever added.
---
 src/modules/netinstall/Config.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 2d663829c..135e28c27 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -152,6 +152,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     if ( m_queue && m_queue->count() > 0 )
     {
         cDebug() << "Loading netinstall from" << m_queue->count() << "alternate sources.";
+        setStatus( required() ? Status::FailedNoData : Status::Ok );
         connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
         m_queue->load();
     }

From a21665011f8645470cf3708789bd4ff65228d71e Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Wed, 14 Apr 2021 12:00:02 +0200
Subject: [PATCH 04/13] [netinstall] The status is ready (done) when the queue
 is done

- Don't signal ready every time data is sent to the model, since
  if the model ends up empty, loading will continue with the next
  fallback entry.
---
 src/modules/netinstall/Config.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 135e28c27..1656f7a06 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -97,7 +97,6 @@ Config::loadGroupList( const QVariantList& groupData )
     {
         setStatus( Status::Ok );
     }
-    emit statusReady();
 }
 
 void
@@ -108,6 +107,7 @@ Config::loadingDone()
         m_queue->deleteLater();
         m_queue = nullptr;
     }
+    emit statusReady();
 }
 
 

From dfedc0fb21f93052b8d2ecb93ebd8285737011a5 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Wed, 14 Apr 2021 12:06:33 +0200
Subject: [PATCH 05/13] [netinstall] Extend tests

- add an "empty" groups file
- run an event loop to give the loader the opportunity to load
---
 src/modules/netinstall/Tests.cpp                | 17 +++++++++++++----
 .../netinstall/tests/1a-single-empty.conf       |  7 +++++++
 src/modules/netinstall/tests/data-empty.yaml    |  6 ++++++
 3 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100644 src/modules/netinstall/tests/1a-single-empty.conf
 create mode 100644 src/modules/netinstall/tests/data-empty.yaml

diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index b8826c6be..090355470 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -342,8 +342,9 @@ ItemTests::testUrlFallback_data()
 
     using S = Config::Status;
 
-    QTest::newRow( "first" ) << "tests/1a-single-bad.conf" << smash( S::FailedNoData ) << 0;
-    QTest::newRow( "second" ) << "tests/1b-single-small.conf" << smash( S::Ok ) << 2;
+    QTest::newRow( "bad" ) << "1a-single-bad.conf" << smash( S::FailedBadData ) << 0;
+    QTest::newRow( "empty" ) << "1a-single-empty.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "second" ) << "1b-single-small.conf" << smash( S::Ok ) << 2;
 }
 
 void
@@ -357,7 +358,8 @@ ItemTests::testUrlFallback()
     cDebug() << "Loading" << filename;
 
     // BUILD_AS_TEST is the source-directory path
-    QFile fi( QString( "%1/%2" ).arg( BUILD_AS_TEST, filename ) );
+    QString testdir = QString( "%1/tests" ).arg( BUILD_AS_TEST );
+    QFile fi( QString( "%1/%2" ).arg( testdir, filename ) );
     QVERIFY( fi.exists() );
 
     Config c;
@@ -368,7 +370,7 @@ ItemTests::testUrlFallback()
         QString ba( yamlFile.readAll() );
         QVERIFY( ba.length() > 0 );
         QHash< QString, QString > replace;
-        replace.insert( "TESTDIR", BUILD_AS_TEST );
+        replace.insert( "TESTDIR", testdir );
         QString correctedDocument = KMacroExpander::expandMacros( ba, replace, '$' );
 
         try
@@ -391,6 +393,13 @@ ItemTests::testUrlFallback()
 
     // Each of the configs sets required to **true**, which is not the default
     QVERIFY( c.required() );
+
+    // Now give the loader time to complete
+    QEventLoop loop;
+    connect( &c, &Config::statusReady, &loop, &QEventLoop::quit );
+    QTimer::singleShot( std::chrono::seconds(1), &loop, &QEventLoop::quit );
+    loop.exec();
+
     QCOMPARE( smash( c.statusCode() ), status );
     QCOMPARE( c.model()->rowCount(), count );
 }
diff --git a/src/modules/netinstall/tests/1a-single-empty.conf b/src/modules/netinstall/tests/1a-single-empty.conf
new file mode 100644
index 000000000..2444a0435
--- /dev/null
+++ b/src/modules/netinstall/tests/1a-single-empty.conf
@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-empty.yaml
diff --git a/src/modules/netinstall/tests/data-empty.yaml b/src/modules/netinstall/tests/data-empty.yaml
new file mode 100644
index 000000000..065a0a067
--- /dev/null
+++ b/src/modules/netinstall/tests/data-empty.yaml
@@ -0,0 +1,6 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+bogus: true
+

From bd118bb457708c86a60a6b3debf931ea1fcce837 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Wed, 14 Apr 2021 12:14:24 +0200
Subject: [PATCH 06/13] [netinstall] Massage test data

- hidden groups aren't counted at all
- count() at top-level of the model counts groups
---
 src/modules/netinstall/tests/data-small.yaml | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/modules/netinstall/tests/data-small.yaml b/src/modules/netinstall/tests/data-small.yaml
index 8e92736b2..6554cf738 100644
--- a/src/modules/netinstall/tests/data-small.yaml
+++ b/src/modules/netinstall/tests/data-small.yaml
@@ -1,12 +1,17 @@
 # SPDX-FileCopyrightText: no
 # SPDX-License-Identifier: CC0-1.0
 #
----
 - name: "Default"
   description: "Default group"
-  hidden: true
+  hidden: false
   selected: true
   critical: false
   packages:
     - base
+- name: "Second"
+  description: "Second group"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
     - chakra-live-skel

From 67effe421490e76696533c7804650e98df596d0a Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Wed, 14 Apr 2021 13:04:40 +0200
Subject: [PATCH 07/13] [netinstall] check in test that loading did not time
 out

---
 src/modules/netinstall/Tests.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index 090355470..392faa34a 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -397,9 +397,13 @@ ItemTests::testUrlFallback()
     // Now give the loader time to complete
     QEventLoop loop;
     connect( &c, &Config::statusReady, &loop, &QEventLoop::quit );
+    QSignalSpy spy( &c, &Config::statusReady );
     QTimer::singleShot( std::chrono::seconds(1), &loop, &QEventLoop::quit );
     loop.exec();
 
+    // Check it didn't time out
+    QCOMPARE( spy.count(), 1 );
+    // Check YAML-loading results
     QCOMPARE( smash( c.statusCode() ), status );
     QCOMPARE( c.model()->rowCount(), count );
 }

From bd2fb552b50b589dc14191ee51e71fa80a86809b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 16 Apr 2021 11:20:04 +0200
Subject: [PATCH 08/13] [netinstall] let queue finish properly

- if the queue is emptied, there was no usable data; set
  failure to NoData rather than BadData.
- FetchNextUnless::done() is done only if the parameter is true (that
  is, it's done!); otherwise should continue.
---
 src/modules/netinstall/LoaderQueue.cpp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/modules/netinstall/LoaderQueue.cpp b/src/modules/netinstall/LoaderQueue.cpp
index 76307d380..0644514c3 100644
--- a/src/modules/netinstall/LoaderQueue.cpp
+++ b/src/modules/netinstall/LoaderQueue.cpp
@@ -26,8 +26,9 @@
  * the queue continues loading. Calling release() before the
  * destructor skips the fetchNext(), ending the queue-loading.
  *
- * Calling done(b) is the same as release(), **plus** done()
- * is called on the queue if @p b is @c true.
+ * Calling done(b) is a conditional release: if @p b is @c true,
+ * queues a call to done() on the queue and releases it; otherwise,
+ * does nothing.
  */
 class FetchNextUnless
 {
@@ -46,11 +47,14 @@ public:
     void release() { m_q = nullptr; }
     void done( bool b )
     {
-        if ( b && m_q )
+        if ( b )
         {
-            QMetaObject::invokeMethod( m_q, "done", Qt::QueuedConnection );
+            if ( m_q )
+            {
+                QMetaObject::invokeMethod( m_q, "done", Qt::QueuedConnection );
+            }
+            release();
         }
-        release();
     }
 
 private:
@@ -94,7 +98,7 @@ LoaderQueue::fetchNext()
 {
     if ( m_queue.isEmpty() )
     {
-        m_config->setStatus( Config::Status::FailedBadData );
+        m_config->setStatus( Config::Status::FailedNoData );
         emit done();
         return;
     }

From 850825f70fb6953ffc775f8c49c6792cd538aa10 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 16 Apr 2021 11:32:04 +0200
Subject: [PATCH 09/13] [netinstall] Leave the last status on the queue

- Reaching the end means there's no data, but leave the last load result
  (presumably bad-something) around rather than overwriting.
---
 src/modules/netinstall/LoaderQueue.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/modules/netinstall/LoaderQueue.cpp b/src/modules/netinstall/LoaderQueue.cpp
index 0644514c3..50b3354ba 100644
--- a/src/modules/netinstall/LoaderQueue.cpp
+++ b/src/modules/netinstall/LoaderQueue.cpp
@@ -98,7 +98,6 @@ LoaderQueue::fetchNext()
 {
     if ( m_queue.isEmpty() )
     {
-        m_config->setStatus( Config::Status::FailedNoData );
         emit done();
         return;
     }

From 9569105575804aea7186b5f2c40cccc169ec0cfe Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 16 Apr 2021 11:32:34 +0200
Subject: [PATCH 10/13] [netinstall] Extend tests with YAML syntax error and
 no-files-at-all

---
 src/modules/netinstall/Tests.cpp                  | 4 +++-
 src/modules/netinstall/tests/1a-single-error.conf | 7 +++++++
 src/modules/netinstall/tests/1c-none.conf         | 6 ++++++
 src/modules/netinstall/tests/data-error.yaml      | 5 +++++
 4 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 src/modules/netinstall/tests/1a-single-error.conf
 create mode 100644 src/modules/netinstall/tests/1c-none.conf
 create mode 100644 src/modules/netinstall/tests/data-error.yaml

diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index 392faa34a..9a02d4969 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -342,9 +342,11 @@ ItemTests::testUrlFallback_data()
 
     using S = Config::Status;
 
-    QTest::newRow( "bad" ) << "1a-single-bad.conf" << smash( S::FailedBadData ) << 0;
+    QTest::newRow( "bad" ) << "1a-single-bad.conf" << smash( S::FailedBadConfiguration ) << 0;
     QTest::newRow( "empty" ) << "1a-single-empty.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "error" ) << "1a-single-error.conf" << smash( S::FailedBadData ) << 0;
     QTest::newRow( "second" ) << "1b-single-small.conf" << smash( S::Ok ) << 2;
+    QTest::newRow( "none" ) << "1c-none.conf" << smash( S::FailedNoData ) << 0;
 }
 
 void
diff --git a/src/modules/netinstall/tests/1a-single-error.conf b/src/modules/netinstall/tests/1a-single-error.conf
new file mode 100644
index 000000000..a602b17e1
--- /dev/null
+++ b/src/modules/netinstall/tests/1a-single-error.conf
@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-error.yaml
diff --git a/src/modules/netinstall/tests/1c-none.conf b/src/modules/netinstall/tests/1c-none.conf
new file mode 100644
index 000000000..e0f097dcf
--- /dev/null
+++ b/src/modules/netinstall/tests/1c-none.conf
@@ -0,0 +1,6 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl: []
diff --git a/src/modules/netinstall/tests/data-error.yaml b/src/modules/netinstall/tests/data-error.yaml
new file mode 100644
index 000000000..fd445df8f
--- /dev/null
+++ b/src/modules/netinstall/tests/data-error.yaml
@@ -0,0 +1,5 @@
+derp
+derp
+herpa-derp: no
+--
+# This file is not valid YAML

From 4dd6ecd54e30ba705d55d42f9cae52be4a512b3d Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 16 Apr 2021 11:39:00 +0200
Subject: [PATCH 11/13] [netinstall] Edge cases of zero, or unset, groups urls

- consumers may wait for loadingDone(), so always emit that
  even if no URL list is set.
---
 src/modules/netinstall/Config.cpp          | 15 ++++++---------
 src/modules/netinstall/Tests.cpp           |  1 +
 src/modules/netinstall/tests/1c-unset.conf |  5 +++++
 3 files changed, 12 insertions(+), 9 deletions(-)
 create mode 100644 src/modules/netinstall/tests/1c-unset.conf

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 1656f7a06..c163d72a0 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -136,26 +136,23 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     // Lastly, load the groups data
     const QString key = QStringLiteral( "groupsUrl" );
     const auto& groupsUrlVariant = configurationMap.value( key );
+    m_queue = new LoaderQueue( this );
     if ( groupsUrlVariant.type() == QVariant::String )
     {
-        m_queue = new LoaderQueue( this );
         m_queue->append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
     }
     else if ( groupsUrlVariant.type() == QVariant::List )
     {
-        m_queue = new LoaderQueue( this );
         for ( const auto& s : groupsUrlVariant.toStringList() )
         {
             m_queue->append( SourceItem::makeSourceItem( s, configurationMap ) );
         }
     }
-    if ( m_queue && m_queue->count() > 0 )
-    {
-        cDebug() << "Loading netinstall from" << m_queue->count() << "alternate sources.";
-        setStatus( required() ? Status::FailedNoData : Status::Ok );
-        connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
-        m_queue->load();
-    }
+
+    setStatus( required() ? Status::FailedNoData : Status::Ok );
+    cDebug() << "Loading netinstall from" << m_queue->count() << "alternate sources.";
+    connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
+    m_queue->load();
 }
 
 void
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index 9a02d4969..06223a709 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -347,6 +347,7 @@ ItemTests::testUrlFallback_data()
     QTest::newRow( "error" ) << "1a-single-error.conf" << smash( S::FailedBadData ) << 0;
     QTest::newRow( "second" ) << "1b-single-small.conf" << smash( S::Ok ) << 2;
     QTest::newRow( "none" ) << "1c-none.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "unset" ) << "1c-unset.conf" << smash( S::FailedNoData ) << 0;
 }
 
 void
diff --git a/src/modules/netinstall/tests/1c-unset.conf b/src/modules/netinstall/tests/1c-unset.conf
new file mode 100644
index 000000000..b25dbb6ea
--- /dev/null
+++ b/src/modules/netinstall/tests/1c-unset.conf
@@ -0,0 +1,5 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true

From 21d24eeb8d5948124be865f9fc5d6ae299dcd85b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 16 Apr 2021 11:47:37 +0200
Subject: [PATCH 12/13] [netinstall] Add tests for fallback loading

- first success that has data is kept
---
 src/modules/netinstall/Tests.cpp              |  5 ++-
 .../netinstall/tests/1b-single-large.conf     |  7 ++++
 .../netinstall/tests/1d-fallback-large.conf   | 10 +++++
 .../netinstall/tests/1d-fallback-small.conf   | 10 +++++
 src/modules/netinstall/tests/data-large.yaml  | 38 +++++++++++++++++++
 5 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 src/modules/netinstall/tests/1b-single-large.conf
 create mode 100644 src/modules/netinstall/tests/1d-fallback-large.conf
 create mode 100644 src/modules/netinstall/tests/1d-fallback-small.conf
 create mode 100644 src/modules/netinstall/tests/data-large.yaml

diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index 06223a709..ff7d8b253 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -345,9 +345,12 @@ ItemTests::testUrlFallback_data()
     QTest::newRow( "bad" ) << "1a-single-bad.conf" << smash( S::FailedBadConfiguration ) << 0;
     QTest::newRow( "empty" ) << "1a-single-empty.conf" << smash( S::FailedNoData ) << 0;
     QTest::newRow( "error" ) << "1a-single-error.conf" << smash( S::FailedBadData ) << 0;
-    QTest::newRow( "second" ) << "1b-single-small.conf" << smash( S::Ok ) << 2;
+    QTest::newRow( "two" ) << "1b-single-small.conf" << smash( S::Ok ) << 2;
+    QTest::newRow( "five" ) << "1b-single-large.conf" << smash( S::Ok ) << 5;
     QTest::newRow( "none" ) << "1c-none.conf" << smash( S::FailedNoData ) << 0;
     QTest::newRow( "unset" ) << "1c-unset.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "fallback-small" ) << "1d-fallback-small.conf" << smash( S::Ok ) << 2;
+    QTest::newRow( "fallback-large" ) << "1d-fallback-large.conf" << smash( S::Ok ) << 5;
 }
 
 void
diff --git a/src/modules/netinstall/tests/1b-single-large.conf b/src/modules/netinstall/tests/1b-single-large.conf
new file mode 100644
index 000000000..eee67e664
--- /dev/null
+++ b/src/modules/netinstall/tests/1b-single-large.conf
@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-large.yaml
diff --git a/src/modules/netinstall/tests/1d-fallback-large.conf b/src/modules/netinstall/tests/1d-fallback-large.conf
new file mode 100644
index 000000000..5abb05ca8
--- /dev/null
+++ b/src/modules/netinstall/tests/1d-fallback-large.conf
@@ -0,0 +1,10 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-bad.yaml
+    - file://$TESTDIR/data-large.yaml
+    - file://$TESTDIR/data-small.yaml
diff --git a/src/modules/netinstall/tests/1d-fallback-small.conf b/src/modules/netinstall/tests/1d-fallback-small.conf
new file mode 100644
index 000000000..e38a7d65f
--- /dev/null
+++ b/src/modules/netinstall/tests/1d-fallback-small.conf
@@ -0,0 +1,10 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-bad.yaml
+    - file://$TESTDIR/data-small.yaml
+    - file://$TESTDIR/data-large.yaml
diff --git a/src/modules/netinstall/tests/data-large.yaml b/src/modules/netinstall/tests/data-large.yaml
new file mode 100644
index 000000000..7b47aa3b6
--- /dev/null
+++ b/src/modules/netinstall/tests/data-large.yaml
@@ -0,0 +1,38 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+- name: "Default"
+  description: "Default group"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - base
+- name: "Two"
+  description: "group 2"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-two
+- name: "Three"
+  description: "group 3"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-three
+- name: "Four"
+  description: "group 4"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-four
+- name: "Five"
+  description: "group 5"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-five

From 165e55986632de5501dd08e09e4cf2a9659f1fd8 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 16 Apr 2021 11:54:18 +0200
Subject: [PATCH 13/13] [netinstall] Extend tests with mixed fallbacks

- insert bad or empty URLs in between successful loads,
  check tail end of loading process.
---
 src/modules/netinstall/Tests.cpp                    |  6 ++++++
 src/modules/netinstall/tests/1d-fallback-bad.conf   | 10 ++++++++++
 src/modules/netinstall/tests/1d-fallback-mixed.conf | 13 +++++++++++++
 3 files changed, 29 insertions(+)
 create mode 100644 src/modules/netinstall/tests/1d-fallback-bad.conf
 create mode 100644 src/modules/netinstall/tests/1d-fallback-mixed.conf

diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index ff7d8b253..9f38f6fbf 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -349,8 +349,14 @@ ItemTests::testUrlFallback_data()
     QTest::newRow( "five" ) << "1b-single-large.conf" << smash( S::Ok ) << 5;
     QTest::newRow( "none" ) << "1c-none.conf" << smash( S::FailedNoData ) << 0;
     QTest::newRow( "unset" ) << "1c-unset.conf" << smash( S::FailedNoData ) << 0;
+    // Finds small, then stops
     QTest::newRow( "fallback-small" ) << "1d-fallback-small.conf" << smash( S::Ok ) << 2;
+    // Finds large, then stops
     QTest::newRow( "fallback-large" ) << "1d-fallback-large.conf" << smash( S::Ok ) << 5;
+    // Finds empty, finds small
+    QTest::newRow( "fallback-mixed" ) << "1d-fallback-mixed.conf" << smash( S::Ok ) << 2;
+    // Finds empty, then bad
+    QTest::newRow( "fallback-bad" ) << "1d-fallback-bad.conf" << smash( S::FailedBadConfiguration ) << 0;
 }
 
 void
diff --git a/src/modules/netinstall/tests/1d-fallback-bad.conf b/src/modules/netinstall/tests/1d-fallback-bad.conf
new file mode 100644
index 000000000..1a36f7854
--- /dev/null
+++ b/src/modules/netinstall/tests/1d-fallback-bad.conf
@@ -0,0 +1,10 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-bad.yaml
diff --git a/src/modules/netinstall/tests/1d-fallback-mixed.conf b/src/modules/netinstall/tests/1d-fallback-mixed.conf
new file mode 100644
index 000000000..79cf677f9
--- /dev/null
+++ b/src/modules/netinstall/tests/1d-fallback-mixed.conf
@@ -0,0 +1,13 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-bad.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-small.yaml
+    - file://$TESTDIR/data-large.yaml
+    - file://$TESTDIR/data-bad.yaml