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/Config.cpp b/src/modules/netinstall/Config.cpp index 2d663829c..c163d72a0 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(); } @@ -136,25 +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."; - 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/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..50b3354ba 100644 --- a/src/modules/netinstall/LoaderQueue.cpp +++ b/src/modules/netinstall/LoaderQueue.cpp @@ -25,6 +25,10 @@ * 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 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 { @@ -41,6 +45,17 @@ public: } } void release() { m_q = nullptr; } + void done( bool b ) + { + if ( b ) + { + if ( m_q ) + { + QMetaObject::invokeMethod( m_q, "done", Qt::QueuedConnection ); + } + release(); + } + } private: LoaderQueue* m_q = nullptr; @@ -83,7 +98,6 @@ LoaderQueue::fetchNext() { if ( m_queue.isEmpty() ) { - m_config->setStatus( Config::Status::FailedBadData ); emit done(); return; } @@ -138,7 +152,7 @@ LoaderQueue::fetch( const QUrl& url ) void LoaderQueue::dataArrived() { - FetchNextUnless finished( this ); + FetchNextUnless next( this ); if ( !m_reply || !m_reply->isFinished() ) { @@ -170,16 +184,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 { diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp index 0b59658c1..9f38f6fbf 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 + #include 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,93 @@ ItemTests::testExampleFiles() } } +void +ItemTests::testUrlFallback_data() +{ + QTest::addColumn< QString >( "filename" ); + QTest::addColumn< int >( "status" ); + QTest::addColumn< int >( "count" ); + + using S = Config::Status; + + 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( "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; + // 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 +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 + QString testdir = QString( "%1/tests" ).arg( BUILD_AS_TEST ); + QFile fi( QString( "%1/%2" ).arg( testdir, 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", testdir ); + 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() ); + + // 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 ); +} + 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/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/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/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/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/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/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 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-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-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 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-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 + 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 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 diff --git a/src/modules/netinstall/tests/data-small.yaml b/src/modules/netinstall/tests/data-small.yaml new file mode 100644 index 000000000..6554cf738 --- /dev/null +++ b/src/modules/netinstall/tests/data-small.yaml @@ -0,0 +1,17 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +# +- name: "Default" + description: "Default group" + hidden: false + selected: true + critical: false + packages: + - base +- name: "Second" + description: "Second group" + hidden: false + selected: true + critical: false + packages: + - chakra-live-skel