From c64aefe43c50bfafc2fd5f50fb60bfc7bb11d09a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 6 Aug 2020 22:50:11 +0200 Subject: [PATCH 01/11] [libcalamares] Remove unused include, declaration --- src/libcalamares/GlobalStorage.cpp | 1 - src/libcalamares/GlobalStorage.h | 4 ---- 2 files changed, 5 deletions(-) diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index 341fc3892..07af5e1b1 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -22,7 +22,6 @@ */ #include "GlobalStorage.h" -#include "JobQueue.h" #include "utils/Logger.h" #include "utils/Units.h" diff --git a/src/libcalamares/GlobalStorage.h b/src/libcalamares/GlobalStorage.h index a2848f888..b242cc205 100644 --- a/src/libcalamares/GlobalStorage.h +++ b/src/libcalamares/GlobalStorage.h @@ -24,8 +24,6 @@ #ifndef CALAMARES_GLOBALSTORAGE_H #define CALAMARES_GLOBALSTORAGE_H -#include "CalamaresConfig.h" - #include #include #include @@ -33,8 +31,6 @@ namespace Calamares { -class DebugWindow; - class GlobalStorage : public QObject { Q_OBJECT From 527449a10215d78647089c6bb37520063b16c41f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 6 Aug 2020 22:54:37 +0200 Subject: [PATCH 02/11] [libcalamares] Improve GS debugDump() formatting --- src/libcalamares/GlobalStorage.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index 07af5e1b1..f9716e661 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -88,9 +88,10 @@ GlobalStorage::value( const QString& key ) const void GlobalStorage::debugDump() const { + cDebug() << "GlobalStorage" << Logger::Pointer(this) << m.count() << "items"; for ( auto it = m.cbegin(); it != m.cend(); ++it ) { - cDebug() << it.key() << '\t' << it.value(); + cDebug() << Logger::SubEntry << it.key() << '\t' << it.value(); } } From 104452513bbb469d576c3cc9b8b1d9750b3b9342 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 6 Aug 2020 23:14:44 +0200 Subject: [PATCH 03/11] [libcalamares] Document GS - write apidox for all of GlobalStorage - while here, polish up the SPDX bits --- src/libcalamares/GlobalStorage.cpp | 4 +- src/libcalamares/GlobalStorage.h | 91 +++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index f9716e661..519b1d0d6 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -2,6 +2,7 @@ * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "GlobalStorage.h" diff --git a/src/libcalamares/GlobalStorage.h b/src/libcalamares/GlobalStorage.h index b242cc205..782d443d4 100644 --- a/src/libcalamares/GlobalStorage.h +++ b/src/libcalamares/GlobalStorage.h @@ -2,6 +2,7 @@ * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef CALAMARES_GLOBALSTORAGE_H @@ -31,21 +29,65 @@ namespace Calamares { +/** @brief Storage for data that passes between Calamares modules. + * + * The Global Storage is global to the Calamars JobQueue and + * everything that depends on that: all of its modules use the + * same instance of the JobQueue, and so of the Global Storage. + * + * GS is used to pass data between modules; there is only convention + * about what keys are used, and individual modules should document + * what they put in to GS or what they expect to find in it. + * + * GS behaves as a basic key-value store, with a QVariantMap behind + * it. Any QVariant can be put into the storage, and the signal + * changed() is emitted when any data is modified. + * + * In general, see QVariantMap (possibly after calling data()) for details. + * + * This class is not thread-safe, but as long as JobQueue is, that's ok + * because only one module is active at a time. + */ class GlobalStorage : public QObject { Q_OBJECT public: + /** @brief Create a GS object + * + * **Generally** there is only one GS object (hence, "global") which + * is owned by the JobQueue instance (which is a singleton). However, + * it is possible to create more GS objects. + */ explicit GlobalStorage( QObject* parent = nullptr ); - //NOTE: thread safety is guaranteed by JobQueue, which executes jobs one by one. - // If at any time jobs become concurrent, this class must be made thread-safe. + /** @brief Insert a key and value into the store + * + * The @p value is added to the store with key @p key. If @p key + * already exists in the store, its existing value is overwritten. + * The changed() signal is emitted regardless. + */ void insert( const QString& key, const QVariant& value ); + /** @brief Removes a key and its value + * + * The @p key is removed from the store. If the @p key does not + * exist, nothing happens. changed() is emitted regardless. + * + * @return the number of keys remaining + */ int remove( const QString& key ); - /// @brief dump keys and values to the debug log + /** @brief dump keys and values to the debug log + * + * All the keys and their values are written to the debug log. + * See save() for caveats: this can leak sensitive information. + */ void debugDump() const; /** @brief write as JSON to the given filename + * + * The file named @p filename is overwritten with a JSON representation + * of the entire global storage (this may be structured, for instance + * if maps or lists have been inserted). * * No tidying, sanitization, or censoring is done -- for instance, * the user module sets a slightly-obscured password in global storage, @@ -58,7 +100,8 @@ public: * * No tidying, sanitization, or censoring is done. * The JSON file is read and each key is added as a value to - * the global storage. + * the global storage. The storage is not cleared first: existing + * keys will remain; keys that also occur in the JSON file are overwritten. */ bool load( const QString& filename ); @@ -68,7 +111,10 @@ public: */ bool saveYaml( const QString& filename ); - /// @brief reads settings from the given filename + /** @brief reads settings from the given filename + * + * See also load(), above. + */ bool loadYaml( const QString& filename ); /** @brief Get internal mapping as a constant object @@ -80,12 +126,41 @@ public: const QVariantMap& data() const { return m; } public Q_SLOTS: + /** @brief Does the store contain the given key? + * + * This can distinguish an explicitly-inserted QVariant() from + * a no-value-exists QVariant. See value() for details. + */ bool contains( const QString& key ) const; + /** @brief The number of keys in the store + * + * This should be unsigned, but the underlying QMap uses signed as well. + * Equal to keys().length(), in theory. + */ int count() const; + /** @brief The keys in the store + * + * This makes a copy of all the keys currently in the store, which + * could be used for iterating over all the values in the store. + */ QStringList keys() const; + /** @brief Gets a value from the store + * + * If a value has been previously inserted, returns that value. + * If @p key does not exist in the store, returns a QVariant() + * (an invalid QVariant, which boolean-converts to false). Since + * QVariant() van also be inserted explicitly, use contains() + * to check for the presence of a key if you need that. + */ QVariant value( const QString& key ) const; signals: + /** @brief Emitted any time the store changes + * + * Also emitted sometimes when the store does not change, e.g. + * when removing a non-existent key or inserting a value that + * is already present. + */ void changed(); private: From 0121e3755b6cda034ecf759a12a7ed477617e0dc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Aug 2020 00:01:20 +0200 Subject: [PATCH 04/11] [libcalamares] GS improve load/save - save should be const - rename save() to saveJson() for parity with saveYaml() --- src/libcalamares/GlobalStorage.cpp | 6 +++--- src/libcalamares/GlobalStorage.h | 6 +++--- src/modules/preservefiles/PreserveFiles.cpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index 519b1d0d6..a31a89b21 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -94,7 +94,7 @@ GlobalStorage::debugDump() const } bool -GlobalStorage::save( const QString& filename ) +GlobalStorage::saveJson( const QString& filename ) const { QFile f( filename ); if ( !f.open( QFile::WriteOnly ) ) @@ -108,7 +108,7 @@ GlobalStorage::save( const QString& filename ) } bool -GlobalStorage::load( const QString& filename ) +GlobalStorage::loadJson( const QString& filename ) { QFile f( filename ); if ( !f.open( QFile::ReadOnly ) ) @@ -139,7 +139,7 @@ GlobalStorage::load( const QString& filename ) } bool -GlobalStorage::saveYaml( const QString& filename ) +GlobalStorage::saveYaml( const QString& filename ) const { return CalamaresUtils::saveYaml( filename, m ); } diff --git a/src/libcalamares/GlobalStorage.h b/src/libcalamares/GlobalStorage.h index 782d443d4..14d5d41e9 100644 --- a/src/libcalamares/GlobalStorage.h +++ b/src/libcalamares/GlobalStorage.h @@ -94,7 +94,7 @@ public: * and this JSON file will contain that password in-the-only-slightly- * obscured form. */ - bool save( const QString& filename ); + bool saveJson( const QString& filename ) const; /** @brief Adds the keys from the given JSON file * @@ -103,13 +103,13 @@ public: * the global storage. The storage is not cleared first: existing * keys will remain; keys that also occur in the JSON file are overwritten. */ - bool load( const QString& filename ); + bool loadJson( const QString& filename ); /** @brief write as YAML to the given filename * * See also save(), above. */ - bool saveYaml( const QString& filename ); + bool saveYaml( const QString& filename ) const; /** @brief reads settings from the given filename * diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index 8352e861d..7262630e0 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -138,7 +138,7 @@ PreserveFiles::exec() } if ( it.type == ItemType::Config ) { - if ( Calamares::JobQueue::instance()->globalStorage()->save( dest ) ) + if ( Calamares::JobQueue::instance()->globalStorage()->saveJson( dest ) ) { cWarning() << "Could not write config for" << dest; } From dc5d98af7d5c9c2b3ff36fa07912abf5f0331a46 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 6 Aug 2020 23:27:04 +0200 Subject: [PATCH 05/11] [libcalamares] Address outdates assumptions about thread-safety --- src/libcalamares/GlobalStorage.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/GlobalStorage.h b/src/libcalamares/GlobalStorage.h index 14d5d41e9..a12d78978 100644 --- a/src/libcalamares/GlobalStorage.h +++ b/src/libcalamares/GlobalStorage.h @@ -22,6 +22,7 @@ #ifndef CALAMARES_GLOBALSTORAGE_H #define CALAMARES_GLOBALSTORAGE_H +#include #include #include #include @@ -45,8 +46,11 @@ namespace Calamares * * In general, see QVariantMap (possibly after calling data()) for details. * - * This class is not thread-safe, but as long as JobQueue is, that's ok - * because only one module is active at a time. + * This class is thread-safe -- most accesses go through JobQueue, which + * handles threading itself, but because modules load in parallel and can + * have asynchronous tasks like GeoIP lookups, the storage itself also + * has locking. All methods are thread-safe, use data() to make a snapshot + * copy for use outside of the thread-safe API. */ class GlobalStorage : public QObject { @@ -117,13 +121,11 @@ public: */ bool loadYaml( const QString& filename ); - /** @brief Get internal mapping as a constant object + /** @brief Make a complete copy of the data * - * Note that the VariantMap underneath may change, because - * it's not constant in itself. Connect to the changed() - * signal for notifications. + * Provides a snapshot of the data at a given time. */ - const QVariantMap& data() const { return m; } + QVariantMap data() const { return m; } public Q_SLOTS: /** @brief Does the store contain the given key? @@ -164,7 +166,10 @@ signals: void changed(); private: + class ReadLock; + class WriteLock; QVariantMap m; + mutable QMutex m_mutex; }; } // namespace Calamares From ac713d8c4bdf07fb80d171a0fa1b6a58517d1c47 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 6 Aug 2020 23:48:18 +0200 Subject: [PATCH 06/11] [libcalamares] Apply locking to GS access --- src/libcalamares/GlobalStorage.cpp | 41 +++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index a31a89b21..6d3b33450 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -27,12 +27,35 @@ #include #include +#include using CalamaresUtils::operator""_MiB; namespace Calamares { +class GlobalStorage::ReadLock : public QMutexLocker +{ +public: + ReadLock( const GlobalStorage* gs ) + : QMutexLocker( &gs->m_mutex ) + { + } +}; + +class GlobalStorage::WriteLock : public QMutexLocker +{ +public: + WriteLock( GlobalStorage* gs ) + : QMutexLocker( &gs->m_mutex ) + , m_gs( gs ) + { + } + ~WriteLock() { m_gs->changed(); } + + GlobalStorage* m_gs; +}; + GlobalStorage::GlobalStorage( QObject* parent ) : QObject( parent ) { @@ -42,6 +65,7 @@ GlobalStorage::GlobalStorage( QObject* parent ) bool GlobalStorage::contains( const QString& key ) const { + ReadLock l( this ); return m.contains( key ); } @@ -49,6 +73,7 @@ GlobalStorage::contains( const QString& key ) const int GlobalStorage::count() const { + ReadLock l( this ); return m.count(); } @@ -56,14 +81,15 @@ GlobalStorage::count() const void GlobalStorage::insert( const QString& key, const QVariant& value ) { + WriteLock l( this ); m.insert( key, value ); - emit changed(); } QStringList GlobalStorage::keys() const { + ReadLock l( this ); return m.keys(); } @@ -71,8 +97,8 @@ GlobalStorage::keys() const int GlobalStorage::remove( const QString& key ) { + WriteLock l( this ); int nItems = m.remove( key ); - emit changed(); return nItems; } @@ -80,13 +106,15 @@ GlobalStorage::remove( const QString& key ) QVariant GlobalStorage::value( const QString& key ) const { + ReadLock l( this ); return m.value( key ); } void GlobalStorage::debugDump() const { - cDebug() << "GlobalStorage" << Logger::Pointer(this) << m.count() << "items"; + ReadLock l( this ); + cDebug() << "GlobalStorage" << Logger::Pointer( this ) << m.count() << "items"; for ( auto it = m.cbegin(); it != m.cend(); ++it ) { cDebug() << Logger::SubEntry << it.key() << '\t' << it.value(); @@ -96,6 +124,7 @@ GlobalStorage::debugDump() const bool GlobalStorage::saveJson( const QString& filename ) const { + ReadLock l( this ); QFile f( filename ); if ( !f.open( QFile::WriteOnly ) ) { @@ -128,6 +157,7 @@ GlobalStorage::loadJson( const QString& filename ) } else { + WriteLock l( this ); auto map = d.toVariant().toMap(); for ( auto i = map.constBegin(); i != map.constEnd(); ++i ) { @@ -141,6 +171,7 @@ GlobalStorage::loadJson( const QString& filename ) bool GlobalStorage::saveYaml( const QString& filename ) const { + ReadLock l( this ); return CalamaresUtils::saveYaml( filename, m ); } @@ -151,9 +182,11 @@ GlobalStorage::loadYaml( const QString& filename ) auto gs = CalamaresUtils::loadYaml( filename, &ok ); if ( ok ) { + WriteLock l( this ); m = gs; + return true; } - return ok; + return false; } From 3c618a9a192bcc39d26bb8ecba7795a41838c076 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Aug 2020 00:08:47 +0200 Subject: [PATCH 07/11] [libcalamares] Fix GS load behavior - the loadJson behavior did too many notifications, and was likely to deadlock; write directly to the map instead and emit only once. - the loadYaml method did something very different from its documentation or intent. --- src/libcalamares/GlobalStorage.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index 6d3b33450..253a4d6ad 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -158,10 +158,13 @@ GlobalStorage::loadJson( const QString& filename ) else { WriteLock l( this ); + // Do **not** use method insert() here, because it would + // recursively lock the mutex, leading to deadlock. Also, + // that would emit changed() for each key. auto map = d.toVariant().toMap(); for ( auto i = map.constBegin(); i != map.constEnd(); ++i ) { - insert( i.key(), *i ); + m.insert( i.key(), *i ); } return true; } @@ -179,11 +182,17 @@ bool GlobalStorage::loadYaml( const QString& filename ) { bool ok = false; - auto gs = CalamaresUtils::loadYaml( filename, &ok ); + auto map = CalamaresUtils::loadYaml( filename, &ok ); if ( ok ) { WriteLock l( this ); - m = gs; + // Do **not** use method insert() here, because it would + // recursively lock the mutex, leading to deadlock. Also, + // that would emit changed() for each key. + for ( auto i = map.constBegin(); i != map.constEnd(); ++i ) + { + m.insert( i.key(), *i ); + } return true; } return false; From a44e6802e5a296f3b5d53a7ebd0d626b8d087d62 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Aug 2020 00:13:58 +0200 Subject: [PATCH 08/11] [libcalamares] Rename tests for consistency --- src/libcalamares/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 8e209f8a3..fef0b157a 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -216,19 +216,19 @@ endforeach() # # calamares_add_test( - libcalamarestest + libcalamaresutilstest SOURCES utils/Tests.cpp ) calamares_add_test( - libcalamarestestpaths + libcalamaresutilspathstest SOURCES utils/TestPaths.cpp ) calamares_add_test( - geoiptest + libcalamaresgeoiptest SOURCES geoip/GeoIPTests.cpp ${geoip_src} From dbc49f001eccb6882c4ab4b4ad8d8186a7fd7664 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Aug 2020 00:45:36 +0200 Subject: [PATCH 09/11] [libcalamares] Test GS - test insert, remove, emitted signals - test loading and saving of YAML and JSON This shows up a big bug in the YAML saving code (which was never used, it seems, anyway) --- src/libcalamares/CMakeLists.txt | 15 +++-- src/libcalamares/Tests.cpp | 115 ++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 src/libcalamares/Tests.cpp diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index fef0b157a..ff341bb56 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -1,6 +1,7 @@ # === This file is part of Calamares - === # # SPDX-FileCopyrightText: 2020 Adriaan de Groot +# SPDX-License-Identifier: GPL-3.0-or-later # # Calamares is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -15,12 +16,12 @@ # You should have received a copy of the GNU General Public License # along with Calamares. If not, see . # -# SPDX-License-Identifier: GPL-3.0-or-later -# License-Filename: LICENSE -# + # # libcalamares is the non-GUI part of Calamares, which includes handling -# translations, configurations, logging, utilities, global storage, and (non-GUI) jobs. +# translations, configurations, logging, utilities, global storage, and +# (non-GUI) jobs. +# add_definitions( -DDLLEXPORT_PRO ) include_directories( ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR} ) @@ -215,6 +216,12 @@ endforeach() ### TESTING # # +calamares_add_test( + libcalamarestest + SOURCES + Tests.cpp +) + calamares_add_test( libcalamaresutilstest SOURCES diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp new file mode 100644 index 000000000..744e56113 --- /dev/null +++ b/src/libcalamares/Tests.cpp @@ -0,0 +1,115 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2018-2020 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + * + */ + +#include "GlobalStorage.h" + +#include +#include +#include + +class TestLibCalamares : public QObject +{ + Q_OBJECT +public: + TestLibCalamares() {} + virtual ~TestLibCalamares() {} + +private Q_SLOTS: + void testGSModify(); + void testGSLoadSave(); +}; + +void +TestLibCalamares::testGSModify() +{ + Calamares::GlobalStorage gs; + QSignalSpy spy( &gs, &Calamares::GlobalStorage::changed ); + + const QString key( "derp" ); + + QCOMPARE( gs.count(), 0 ); + QVERIFY( !gs.contains( key ) ); + + const int value = 17; + gs.insert( key, value ); + QCOMPARE( gs.count(), 1 ); + QVERIFY( gs.contains( key ) ); + QCOMPARE( gs.value( key ).type(), QVariant::Int ); + QCOMPARE( gs.value( key ).toString(), QString( "17" ) ); // It isn't a string, but does convert + QCOMPARE( gs.value( key ).toInt(), value ); + + gs.remove( key ); + QCOMPARE( gs.count(), 0 ); + QVERIFY( !gs.contains( key ) ); + + QCOMPARE( spy.count(), 2 ); // one insert, one remove +} + +void +TestLibCalamares::testGSLoadSave() +{ + Calamares::GlobalStorage gs; + const QString jsonfilename( "gs.test.json" ); + const QString yamlfilename( "gs.test.yaml" ); + + gs.insert( "derp", 17 ); + gs.insert( "cow", "moo" ); + gs.insert( "dwarfs", QStringList { "dopey", "sneezy" } ); + + QCOMPARE( gs.count(), 3 ); + + QVERIFY( gs.saveJson( jsonfilename ) ); + Calamares::GlobalStorage gs2; + QCOMPARE( gs2.count(), 0 ); + QVERIFY( gs2.loadJson( jsonfilename ) ); + QCOMPARE( gs2.count(), 3 ); + QCOMPARE( gs2.data(), gs.data() ); + + QVERIFY( gs.saveYaml( yamlfilename ) ); + Calamares::GlobalStorage gs3; + QCOMPARE( gs3.count(), 0 ); + QVERIFY( gs3.loadYaml( jsonfilename ) ); + QCOMPARE( gs3.count(), 3 ); + QCOMPARE( gs3.data(), gs.data() ); + + // Failures in loading + QVERIFY( !gs3.loadYaml( jsonfilename ) ); + QVERIFY( !gs3.loadJson( yamlfilename ) ); + + Calamares::GlobalStorage gs4; + gs4.insert( "derp", 32 ); + gs4.insert( "dorp", "Varsseveld" ); + QCOMPARE( gs4.count(), 2 ); + QVERIFY( gs4.contains( "dorp" ) ); + QCOMPARE( gs4.value( "derp" ).toInt(), 32 ); + QVERIFY( gs4.loadJson( jsonfilename ) ); + // 3 keys from the file, but one overwrite + QCOMPARE( gs4.count(), 4 ); + QVERIFY( gs4.contains( "dorp" ) ); + QCOMPARE( gs4.value( "derp" ).toInt(), 17 ); // This one was overwritten +} + + +QTEST_GUILESS_MAIN( TestLibCalamares ) + +#include "utils/moc-warnings.h" + +#include "Tests.moc" From 0de98fe4c125098c7e9325fbc4367ab483612e9f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Aug 2020 00:57:30 +0200 Subject: [PATCH 10/11] [libcalamares] Expand YAML testing a little - load/save of a stringlist seems to work --- src/libcalamares/testdata/yaml-list.conf | 8 ++++++++ src/libcalamares/utils/Tests.cpp | 1 + 2 files changed, 9 insertions(+) create mode 100644 src/libcalamares/testdata/yaml-list.conf diff --git a/src/libcalamares/testdata/yaml-list.conf b/src/libcalamares/testdata/yaml-list.conf new file mode 100644 index 000000000..d6992b1fb --- /dev/null +++ b/src/libcalamares/testdata/yaml-list.conf @@ -0,0 +1,8 @@ +# YAML dump +--- +"cow": "moo" +"derp": 17 +"dwarfs": + - "sleepy" + - "sneezy" + - "doc" diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index 300b9b531..5a01848bf 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -148,6 +148,7 @@ LibCalamaresTests::recursiveCompareMap( const QVariantMap& a, const QVariantMap& void LibCalamaresTests::testLoadSaveYamlExtended() { + Logger::setupLogLevel( Logger::LOGDEBUG ); bool loaded_ok; for ( const auto& confname : findConf( QDir( "../src" ) ) ) { From cb20ba6aba632b85302379511b0d5976def995fc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Aug 2020 01:11:14 +0200 Subject: [PATCH 11/11] [libcalamares] More GS load/save testing - failures elsewhere boil down to QStringList is not supported, but a QVariantList of QVariants of QStrings is. --- src/libcalamares/Tests.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 744e56113..bf419d721 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -21,6 +21,8 @@ #include "GlobalStorage.h" +#include "utils/Logger.h" + #include #include #include @@ -35,6 +37,7 @@ public: private Q_SLOTS: void testGSModify(); void testGSLoadSave(); + void testGSLoadSave2(); }; void @@ -107,6 +110,35 @@ TestLibCalamares::testGSLoadSave() QCOMPARE( gs4.value( "derp" ).toInt(), 17 ); // This one was overwritten } +void +TestLibCalamares::testGSLoadSave2() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); + + const QString filename( "../src/libcalamares/testdata/yaml-list.conf" ); + if ( !QFile::exists( filename ) ) + { + return; + } + + Calamares::GlobalStorage gs1; + const QString key( "dwarfs" ); + + QVERIFY( gs1.loadYaml( filename ) ); + QCOMPARE( gs1.count(), 3 ); // From examining the file + QVERIFY( gs1.contains( key ) ); + cDebug() << gs1.value( key ).type() << gs1.value( key ); + QCOMPARE( gs1.value( key ).type(), QVariant::List ); + + const QString yamlfilename( "gs.test.yaml" ); + QVERIFY( gs1.saveYaml( yamlfilename ) ); + + Calamares::GlobalStorage gs2; + QVERIFY( gs2.loadYaml( yamlfilename ) ); + QVERIFY( gs2.contains( key ) ); + QCOMPARE( gs2.value( key ).type(), QVariant::List ); +} + QTEST_GUILESS_MAIN( TestLibCalamares )