Finish unit tests for the DownloadUpdateTask class

logic/updater/DownloadUpdateTask.cpp

@@ -234,15 +234,36 @@ bool DownloadUpdateTask::parseVersionInfo(const QByteArray &data, VersionFileLis
 }
 
 void DownloadUpdateTask::processFileLists()
+{
+	// Create a network job for downloading files.
+	NetJob* netJob = new NetJob("Update Files");
+
+	processFileLists(netJob, m_cVersionFileList, m_nVersionFileList, m_operationList);
+
+	// Add listeners to wait for the downloads to finish.
+	QObject::connect(netJob, &NetJob::succeeded, this, &DownloadUpdateTask::fileDownloadFinished);
+	QObject::connect(netJob, &NetJob::progress, this, &DownloadUpdateTask::fileDownloadProgressChanged);
+	QObject::connect(netJob, &NetJob::failed, this, &DownloadUpdateTask::fileDownloadFailed);
+
+	// Now start the download.
+	setStatus(tr("Downloading %1 update files.").arg(QString::number(netJob->size())));
+	QLOG_DEBUG() << "Begin downloading update files to" << m_updateFilesDir.path();
+	m_filesNetJob.reset(netJob);
+	netJob->start();
+
+	writeInstallScript(m_operationList, PathCombine(m_updateFilesDir.path(), "file_list.xml"));
+}
+
+void DownloadUpdateTask::processFileLists(NetJob *job, const VersionFileList &currentVersion, const VersionFileList &newVersion, DownloadUpdateTask::UpdateOperationList &ops)
 {
 	setStatus(tr("Processing file lists. Figuring out how to install the update."));
 
 	// First, if we've loaded the current version's file list, we need to iterate through it and 
 	// delete anything in the current one version's list that isn't in the new version's list.
-	for (VersionFileEntry entry : m_cVersionFileList)
+	for (VersionFileEntry entry : currentVersion)
 	{
 		bool keep = false;
-		for (VersionFileEntry newEntry : m_nVersionFileList)
+		for (VersionFileEntry newEntry : newVersion)
 		{
 			if (newEntry.path == entry.path)
 			{
@@ -253,14 +274,11 @@ void DownloadUpdateTask::processFileLists()
 		}
 		// If the loop reaches the end and we didn't find a match, delete the file.
 		if(!keep)
-			m_operationList.append(UpdateOperation::DeleteOp(entry.path));
+			ops.append(UpdateOperation::DeleteOp(entry.path));
 	}
 
-	// Create a network job for downloading files.
-	NetJob* netJob = new NetJob("Update Files");
-
 	// Next, check each file in MultiMC's folder and see if we need to update them.
-	for (VersionFileEntry entry : m_nVersionFileList)
+	for (VersionFileEntry entry : newVersion)
 	{
 		// TODO: Let's not MD5sum a ton of files on the GUI thread. We should probably find a way to do this in the background.
 		QString fileMD5;
@@ -287,31 +305,21 @@ void DownloadUpdateTask::processFileLists()
 					// Download it to updatedir/<filepath>-<md5> where filepath is the file's path with slashes replaced by underscores.
 					QString dlPath = PathCombine(m_updateFilesDir.path(), QString(entry.path).replace("/", "_"));
 
+					if (job)
+					{
 						// We need to download the file to the updatefiles folder and add a task to copy it to its install path.
 						auto download = MD5EtagDownload::make(source.url, dlPath);
 						download->m_check_md5 = true;
 						download->m_expected_md5 = entry.md5;
-					netJob->addNetAction(download);
+						job->addNetAction(download);
+					}
 
 					// Now add a copy operation to our operations list to install the file.
-					m_operationList.append(UpdateOperation::CopyOp(dlPath, entry.path, entry.mode));
+					ops.append(UpdateOperation::CopyOp(dlPath, entry.path, entry.mode));
 				}
 			}
 		}
 	}
-
-	// Add listeners to wait for the downloads to finish.
-	QObject::connect(netJob, &NetJob::succeeded, this, &DownloadUpdateTask::fileDownloadFinished);
-	QObject::connect(netJob, &NetJob::progress, this, &DownloadUpdateTask::fileDownloadProgressChanged);
-	QObject::connect(netJob, &NetJob::failed, this, &DownloadUpdateTask::fileDownloadFailed);
-
-	// Now start the download.
-	setStatus(tr("Downloading %1 update files.").arg(QString::number(netJob->size())));
-	QLOG_DEBUG() << "Begin downloading update files to" << m_updateFilesDir.path();
-	m_filesNetJob.reset(netJob);
-	netJob->start();
-
-	writeInstallScript(m_operationList, PathCombine(m_updateFilesDir.path(), "file_list.xml"));
 }
 
 bool DownloadUpdateTask::writeInstallScript(UpdateOperationList& opsList, QString scriptFile)

logic/updater/DownloadUpdateTask.h

@@ -54,13 +54,11 @@ public:
 		QString url;
 		QString compressionType;
 	};
-
 	typedef QList<FileSource> FileSourceList;
 
 	/*!
 	 * Structure that describes an entry in a GoUpdate version's `Files` list.
 	 */
-
 	struct VersionFileEntry
 	{
 		QString path;
@@ -68,12 +66,8 @@ public:
 		FileSourceList sources;
 		QString md5;
 	};
-
 	typedef QList<VersionFileEntry> VersionFileList;
 
-protected:
-	friend class DownloadUpdateTaskTest;
-
 	/*!
 	 * Structure that describes an operation to perform when installing updates.
 	 */
@@ -104,9 +98,12 @@ protected:
 
 		// Yeah yeah, polymorphism blah blah inheritance, blah blah object oriented. I'm lazy, OK?
 	};
-
 	typedef QList<UpdateOperation> UpdateOperationList;
 
+protected:
+	friend class DownloadUpdateTaskTest;
+
+
 	/*!
 	 * Used for arguments to parseVersionInfo and friends to specify which version info file to parse.
 	 */
@@ -159,6 +156,12 @@ protected:
 	 * Takes a list of file entries for the current version's files and the new version's files
 	 * and populates the downloadList and operationList with information about how to download and install the update.
 	 */
+	virtual void processFileLists(NetJob *job, const VersionFileList &currentVersion, const VersionFileList &newVersion, UpdateOperationList &ops);
+
+	/*!
+	 * Calls \see processFileLists to populate the \see m_operationList and a NetJob, and then executes
+	 * the NetJob to fetch all needed files
+	 */
 	virtual void processFileLists();
 
 	/*!
@@ -166,7 +169,6 @@ protected:
 	 */
 	virtual bool writeInstallScript(UpdateOperationList& opsList, QString scriptFile);
 
-	VersionFileList m_downloadList;
 	UpdateOperationList m_operationList;
 
 	VersionFileList m_nVersionFileList;

tests/tst_DownloadUpdateTask.cpp

@@ -5,8 +5,10 @@
 
 #include "logic/updater/DownloadUpdateTask.h"
 #include "logic/updater/UpdateChecker.h"
+#include "depends/util/include/pathutils.h"
 
 Q_DECLARE_METATYPE(DownloadUpdateTask::VersionFileList)
+Q_DECLARE_METATYPE(DownloadUpdateTask::UpdateOperation)
 
 bool operator==(const DownloadUpdateTask::FileSource &f1, const DownloadUpdateTask::FileSource &f2)
 {
@@ -21,6 +23,13 @@ bool operator==(const DownloadUpdateTask::VersionFileEntry &v1, const DownloadUp
 			v1.sources == v2.sources &&
 			v1.md5 == v2.md5;
 }
+bool operator==(const DownloadUpdateTask::UpdateOperation &u1, const DownloadUpdateTask::UpdateOperation &u2)
+{
+	return u1.type == u2.type &&
+			u1.file == u2.file &&
+			u1.dest == u2.dest &&
+			u1.mode == u2.mode;
+}
 
 QDebug operator<<(QDebug dbg, const DownloadUpdateTask::FileSource &f)
 {
@@ -32,6 +41,22 @@ QDebug operator<<(QDebug dbg, const DownloadUpdateTask::VersionFileEntry &v)
 	dbg.nospace() << "VersionFileEntry(path=" << v.path << " mode=" << v.mode << " md5=" << v.md5 << " sources=" << v.sources << ")";
 	return dbg.maybeSpace();
 }
+QDebug operator<<(QDebug dbg, const DownloadUpdateTask::UpdateOperation::Type &t)
+{
+	switch (t)
+	{
+	case DownloadUpdateTask::UpdateOperation::OP_COPY: dbg << "OP_COPY"; break;
+	case DownloadUpdateTask::UpdateOperation::OP_DELETE: dbg << "OP_DELETE"; break;
+	case DownloadUpdateTask::UpdateOperation::OP_MOVE: dbg << "OP_MOVE"; break;
+	case DownloadUpdateTask::UpdateOperation::OP_CHMOD: dbg << "OP_CHMOD"; break;
+	}
+	return dbg.maybeSpace();
+}
+QDebug operator<<(QDebug dbg, const DownloadUpdateTask::UpdateOperation &u)
+{
+	dbg.nospace() << "UpdateOperation(type=" << u.type << " file=" << u.file << " dest=" << u.dest << " mode=" << u.mode << ")";
+	return dbg.maybeSpace();
+}
 
 class DownloadUpdateTaskTest : public QObject
 {
@@ -108,9 +133,48 @@ slots:
 		QCOMPARE(outError, error);
 	}
 
+	void test_processFileLists_data()
+	{
+		QTest::addColumn<DownloadUpdateTask *>("downloader");
+		QTest::addColumn<DownloadUpdateTask::VersionFileList>("currentVersion");
+		QTest::addColumn<DownloadUpdateTask::VersionFileList>("newVersion");
+		QTest::addColumn<DownloadUpdateTask::UpdateOperationList>("expectedOperations");
+
+		DownloadUpdateTask *downloader = new DownloadUpdateTask(QString(), -1);
+
+		// update fileOne, keep fileTwo, remove fileThree
+		QTest::newRow("test 1") << downloader
+				<< (DownloadUpdateTask::VersionFileList()
+					<< DownloadUpdateTask::VersionFileEntry{QFINDTESTDATA("tests/data/fileOne"), 493, DownloadUpdateTask::FileSourceList()
+															<< DownloadUpdateTask::FileSource("http", "http://host/path/fileOne-1"), "9eb84090956c484e32cb6c08455a667b"}
+					<< DownloadUpdateTask::VersionFileEntry{QFINDTESTDATA("tests/data/fileTwo"), 644, DownloadUpdateTask::FileSourceList()
+															<< DownloadUpdateTask::FileSource("http", "http://host/path/fileTwo-1"), "38f94f54fa3eb72b0ea836538c10b043"}
+					<< DownloadUpdateTask::VersionFileEntry{QFINDTESTDATA("tests/data/fileThree"), 420, DownloadUpdateTask::FileSourceList()
+															<< DownloadUpdateTask::FileSource("http", "http://host/path/fileThree-1"), "f12df554b21e320be6471d7154130e70"})
+				<< (DownloadUpdateTask::VersionFileList()
+					<< DownloadUpdateTask::VersionFileEntry{QFINDTESTDATA("tests/data/fileOne"), 493, DownloadUpdateTask::FileSourceList()
+															<< DownloadUpdateTask::FileSource("http", "http://host/path/fileOne-2"), "42915a71277c9016668cce7b82c6b577"}
+					<< DownloadUpdateTask::VersionFileEntry{QFINDTESTDATA("tests/data/fileTwo"), 644, DownloadUpdateTask::FileSourceList()
+															<< DownloadUpdateTask::FileSource("http", "http://host/path/fileTwo-2"), "38f94f54fa3eb72b0ea836538c10b043"})
+				<< (DownloadUpdateTask::UpdateOperationList()
+					<< DownloadUpdateTask::UpdateOperation::DeleteOp(QFINDTESTDATA("tests/data/fileThree"))
+					<< DownloadUpdateTask::UpdateOperation::CopyOp(PathCombine(downloader->updateFilesDir(), QFINDTESTDATA("tests/data/fileOne").replace("/", "_")),
+																   QFINDTESTDATA("tests/data/fileOne"), 493));
+	}
 	void test_processFileLists()
 	{
-		// TODO create unit test for this
+		QFETCH(DownloadUpdateTask *, downloader);
+		QFETCH(DownloadUpdateTask::VersionFileList, currentVersion);
+		QFETCH(DownloadUpdateTask::VersionFileList, newVersion);
+		QFETCH(DownloadUpdateTask::UpdateOperationList, expectedOperations);
+
+		DownloadUpdateTask::UpdateOperationList operations;
+
+		downloader->processFileLists(new NetJob("Dummy"), currentVersion, newVersion, operations);
+		qDebug() << (operations == expectedOperations);
+		qDebug() << operations;
+		qDebug() << expectedOperations;
+		QCOMPARE(operations, expectedOperations);
 	}
 
 	void test_masterTest()