testing/git-storage: fairly fundamental rewrite of the storage test

The old test was broken in many ways and kept failing for a number of
reasons. Some of them were addressed in the previous commits (the
missing HEAD ref being the main one), the other one was that the tests
kept stepping on top of each other - as were potentially random users or
reviewers using the 'universal' test account.

This uses a random one of ten dedicated test accounts, and on top of
that uses a random branch name (instead of the fixed email address
associated with the account).

This also rewrites several of the tests dealing with offline changes to
correctly model going offline (but keeps one that directly writes to the
local cache).

Finally this change also tries to do a much better job cleaning up after
itself and not leaving data behind the the next run could stumble over.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

2 files changed, 174 insertions, 83 deletions

diff --git a/tests/testgitstorage.cpp b/tests/testgitstorage.cpp
index b84892a57..b89c58455 100644
--- a/tests/testgitstorage.cpp
+++ b/tests/testgitstorage.cpp
@@ -16,15 +16,55 @@
 #include <QNetworkProxy>
 #include <QTextCodec>
 #include <QDebug>
+#include <QRandomGenerator>
 
-// this is a local helper function in git-access.c
+// provide declarations for two local helper functions in git-access.c
 extern "C" char *get_local_dir(const char *remote, const char *branch);
+extern "C" void delete_remote_branch(git_repository *repo, const char *remote, const char *branch);
 
 QString email;
 QString gitUrl;
 QString cloudTestRepo;
 QString localCacheDir;
 QString localCacheRepo;
+QString randomBranch;
+
+static void moveDir(QString oldName, QString newName)
+{
+	QDir oldDir(oldName);
+	QDir newDir(newName);
+	QCOMPARE(newDir.removeRecursively(), true);
+	QCOMPARE(oldDir.rename(oldName, newName), true);
+}
+
+static void localRemoteCleanup()
+{
+	// cleanup the local cache dir
+	QDir localCacheDirectory(localCacheDir);
+	QCOMPARE(localCacheDirectory.removeRecursively(), true);
+
+	// get the remote/branch information as parsed by our git tooling
+	const char *branch, *remote;
+	struct git_repository *git_repo;
+
+	// when this is first executed, we expect the branch not to exist on the remote server;
+	// if that's true, this will print a harmless error to stderr
+	git_repo = is_git_repository(qPrintable(cloudTestRepo), &branch, &remote, false);
+
+	// this odd comparison is used to tell that we were able to connect to the remote repo;
+	// in the error case we get the full cloudTestRepo name back as "branch"
+	QCOMPARE(QString(branch), randomBranch);
+
+	// force delete any remote branch of that name on the server (and ignore any errors)
+	delete_remote_branch(git_repo, remote, branch);
+
+	// and since this will have created a local repo, remove that one, again so the tests start clean
+	QCOMPARE(localCacheDirectory.removeRecursively(), true);
+
+	free((void *)branch);
+	free((void *)remote);
+	git_repository_free(git_repo);
+}
 
 void TestGitStorage::initTestCase()
 {
@@ -39,17 +79,19 @@ void TestGitStorage::initTestCase()
 	qPrefProxy::load();
 	qPrefCloudStorage::load();
 
-	// setup our cloud test repo / credentials
-	// allow the caller to overwrite the cloud account we use; this way we
-	// can prevent the GitHub actions running tests in parallel from stepping
-	// on each other. Of course that email needs to exist as cloud storage account
-	// and have the given password
+	// setup our cloud test repo / credentials but allow the user to pick a different account by
+	// setting these environment variables
+	// Of course that email needs to exist as cloud storage account and have the given password
+	//
+	// To reduce the risk of collisions on the server, we have ten accounts set up for this purpose
+	// please don't use them for other reasons as they will get deleted regularly
 	email = qgetenv("SSRF_USER_EMAIL");
 	QString password = qgetenv("SSRF_USER_PASSWORD");
+
 	if (email.isEmpty())
-		email = "ssrftest@hohndel.org";
+		email = QString("gitstorage%1@hohndel.org").arg(QRandomGenerator::global()->bounded(10));
 	if (password.isEmpty())
-		password = "geheim";
+		password = "please-only-use-this-in-the-git-tests";
 	gitUrl = prefs.cloud_base_url;
 	if (gitUrl.right(1) != "/")
 		gitUrl += "/";
@@ -58,13 +100,22 @@ void TestGitStorage::initTestCase()
 	prefs.cloud_storage_email_encoded = copy_qstring(email);
 	prefs.cloud_storage_password = copy_qstring(password);
 	gitUrl += "/" + email;
-	cloudTestRepo = gitUrl + QStringLiteral("[%1]").arg(email);
-	localCacheDir = get_local_dir(qPrintable(gitUrl), qPrintable(email));
-	localCacheRepo = localCacheDir + QStringLiteral("[%1]").arg(email);
-
-	// now cleanup the cache dir in case there's something weird from previous runs
-	QDir localCacheDirectory(localCacheDir);
-	QCOMPARE(localCacheDirectory.removeRecursively(), true);
+	// all user storage for historical reasons always uses the user's email both as
+	// repo name and as branch. To allow us to keep testing and not step on parallel
+	// runs we'll use actually random branch names - yes, this still has a chance of
+	// conflict, but I'm not going to implement a distributed locak manager for this
+	if (email.startsWith("gitstorage")) {
+		randomBranch = QString::number(QRandomGenerator::global()->bounded(0x1000000), 16) +
+				QString::number(QRandomGenerator::global()->bounded(0x1000000), 16);
+	} else {
+		// user supplied their own credentials, fall back to the usual "email is branch" pattern
+		randomBranch = email;
+	}
+	cloudTestRepo = gitUrl + QStringLiteral("[%1]").arg(randomBranch);
+	localCacheDir = get_local_dir(qPrintable(gitUrl), qPrintable(randomBranch));
+	localCacheRepo = localCacheDir + QStringLiteral("[%1]").arg(randomBranch);
+	qDebug() << "repo used:" << cloudTestRepo;
+	qDebug() << "local cache:" << localCacheRepo;
 
 	// make sure we deal with any proxy settings that are needed
 	QNetworkProxy proxy;
@@ -77,8 +128,20 @@ void TestGitStorage::initTestCase()
 	}
 	QNetworkProxy::setApplicationProxy(proxy);
 
-	// make sure that regardless of whether this is a desktop or mobile build, we always check with the cloud
+	// we will keep switching between online and offline mode below; let's always start online
 	git_local_only = false;
+
+	// initialize libgit2
+	git_libgit2_init();
+
+	// cleanup local and remote branches
+	localRemoteCleanup();
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
+}
+
+void TestGitStorage::cleanupTestCase()
+{
+	localRemoteCleanup();
 }
 
 void TestGitStorage::cleanup()
@@ -105,7 +168,6 @@ void TestGitStorage::testGitStorageLocal()
 {
 	// test writing and reading back from local git storage
 	git_repository *repo;
-	git_libgit2_init();
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/SampleDivesV2.ssrf", &dive_table, &trip_table, &dive_site_table), 0);
 	QFETCH(QString, testDirName);
 	QFETCH(QString, prefixRead);
@@ -158,16 +220,18 @@ void TestGitStorage::testGitStorageCloudOfflineSync()
 	// make a change to local cache repo (pretending that we did some offline changes)
 	// and then open the remote one again and check that things were propagated correctly
 	// read the local repo from the previous test and add dive 10
-	QCOMPARE(parse_file(qPrintable(localCacheRepo), &dive_table, &trip_table, &dive_site_table), 0);
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/test10.xml", &dive_table, &trip_table, &dive_site_table), 0);
 	// calling process_loaded_dives() sorts the table, but calling add_imported_dives()
 	// causes it to try to update the window title... let's not do that
 	process_loaded_dives();
 	// now save only to the local cache but not to the remote server
-	QCOMPARE(save_dives(qPrintable(localCacheRepo)), 0);
+	git_local_only = true;
+	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	QCOMPARE(save_dives("./SampleDivesV3plus10local.ssrf"), 0);
 	clear_dive_file_data();
-	// open the cloud storage and compare
+	// now pretend that we are online again and open the cloud storage and compare
+	git_local_only = false;
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(save_dives("./SampleDivesV3plus10viacloud.ssrf"), 0);
 	QFile org("./SampleDivesV3plus10local.ssrf");
@@ -182,10 +246,7 @@ void TestGitStorage::testGitStorageCloudOfflineSync()
 	// write back out to cloud storage, move away the local cache, open again and compare
 	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	clear_dive_file_data();
-	QDir localCacheDirectory(localCacheDir);
-	QDir localCacheDirectorySave(localCacheDir + "save");
-	QCOMPARE(localCacheDirectorySave.removeRecursively(), true);
-	QCOMPARE(localCacheDirectory.rename(localCacheDir, localCacheDir + "save"), true);
+	moveDir(localCacheDir, localCacheDir + "save");
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(save_dives("./SampleDivesV3plus10fromcloud.ssrf"), 0);
 	org.close();
@@ -201,38 +262,56 @@ void TestGitStorage::testGitStorageCloudOfflineSync()
 
 void TestGitStorage::testGitStorageCloudMerge()
 {
-	// now we need to mess with the local git repo to get an actual merge
-	// first we add another dive to the "moved away" repository, pretending we did
-	// another offline change there
-	QString localCacheRepoSave = localCacheDir + QStringLiteral("save[%1]").arg(email);
-	QCOMPARE(parse_file(qPrintable(localCacheRepoSave), &dive_table, &trip_table, &dive_site_table), 0);
+	// we want to test a merge - in order to do so we need to make changes to the cloud
+	// repo from two clients - but since we have only one client here, we have to cheat
+	// a little:
+	// the local cache with the 'save' extension will serve as our second client;
+	//
+	// (1) first we make a change and save it to the cloud
+	// (2) then we switch to the second client (i.e., we move that cache back in place)
+	// (3) on that second client we make a different change while offline
+	// (4) now we take that second client back online and get the merge
+	// (5) let's make sure that we have the expected data on the second client
+	// (6) go back to the first client and ensure we have the same data there after sync
+
+	// (1) open the repo, add dive test11 and save to the cloud
+	git_local_only = false;
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/test11.xml", &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
-	QCOMPARE(save_dives(qPrintable(localCacheRepoSave)), 0);
+	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	clear_dive_file_data();
 
-	// now we open the cloud storage repo and add a different dive to it
+	// (2) switch to the second client by moving the old cache back in place
+	moveDir(localCacheDir, localCacheDir + "client1");
+	moveDir(localCacheDir + "save", localCacheDir);
+
+	// (3) open the repo from the old cache and add dive test12 while offline
+	git_local_only = true;
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/test12.xml", &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
 	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	clear_dive_file_data();
 
-	// now we move the saved local cache into place and try to open the cloud repo
-	// -> this forces a merge
-	QDir localCacheDirectory(localCacheDir);
-	QCOMPARE(localCacheDirectory.removeRecursively(), true);
-	QDir localCacheDirectorySave(localCacheDir + "save");
-	QCOMPARE(localCacheDirectory.rename(localCacheDir + "save", localCacheDir), true);
+	// (4) now take things back online
+	git_local_only = false;
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
-	QCOMPARE(save_dives("./SampleDivesV3plus10-11-12-merged.ssrf"), 0);
 	clear_dive_file_data();
+
+	// (5) now we should have all the dives in our repo on the second client
+	// first create the reference data from the xml files:
 	QCOMPARE(parse_file("./SampleDivesV3plus10local.ssrf", &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/test11.xml", &dive_table, &trip_table, &dive_site_table), 0);
-	process_loaded_dives();
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/test12.xml", &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
 	QCOMPARE(save_dives("./SampleDivesV3plus10-11-12.ssrf"), 0);
+	// then load from the cloud
+	clear_dive_file_data();
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
+	process_loaded_dives();
+	QCOMPARE(save_dives("./SampleDivesV3plus10-11-12-merged.ssrf"), 0);
+	// finally compare what we have
 	QFile org("./SampleDivesV3plus10-11-12-merged.ssrf");
 	org.open(QFile::ReadOnly);
 	QFile out("./SampleDivesV3plus10-11-12.ssrf");
@@ -242,6 +321,18 @@ void TestGitStorage::testGitStorageCloudMerge()
 	QString readin = orgS.readAll();
 	QString written = outS.readAll();
 	QCOMPARE(readin, written);
+	clear_dive_file_data();
+
+	// (6) move ourselves back to the first client and compare data there
+	moveDir(localCacheDir + "client1", localCacheDir);
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
+	process_loaded_dives();
+	QCOMPARE(save_dives("./SampleDivesV3plus10-11-12-merged-client1.ssrf"), 0);
+	QFile client1("./SampleDivesV3plus10-11-12-merged-client1.ssrf");
+	client1.open(QFile::ReadOnly);
+	QTextStream client1S(&client1);
+	readin = client1S.readAll();
+	QCOMPARE(readin, written);
 }
 
 void TestGitStorage::testGitStorageCloudMerge2()
@@ -249,22 +340,21 @@ void TestGitStorage::testGitStorageCloudMerge2()
 	// delete a dive offline
 	// edit the same dive in the cloud repo
 	// merge
-	QCOMPARE(parse_file(qPrintable(localCacheRepo), &dive_table, &trip_table, &dive_site_table), 0);
+	// (1) open repo, delete second dive, save offline
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
 	struct dive *dive = get_dive(1);
 	delete_single_dive(1);
 	QCOMPARE(save_dives("./SampleDivesMinus1.ssrf"), 0);
+	git_local_only = true;
 	QCOMPARE(save_dives(qPrintable(localCacheRepo)), 0);
+	git_local_only = false;
 	clear_dive_file_data();
 
-	// move the local cache away
-	{ // scope for variables
-		QDir localCacheDirectory(localCacheDir);
-		QDir localCacheDirectorySave(localCacheDir + "save");
-		QCOMPARE(localCacheDirectorySave.removeRecursively(), true);
-		QCOMPARE(localCacheDirectory.rename(localCacheDir, localCacheDir + "save"), true);
-	}
-	// now we open the cloud storage repo and modify that first dive
+	// (2) move cache out of the way
+	moveDir(localCacheDir, localCacheDir + "save");
+
+	// (3) now we open the cloud storage repo and modify that second dive
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
 	dive = get_dive(1);
@@ -274,13 +364,9 @@ void TestGitStorage::testGitStorageCloudMerge2()
 	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	clear_dive_file_data();
 
-	// now we move the saved local cache into place and try to open the cloud repo
-	// -> this forces a merge
-	QDir localCacheDirectory(localCacheDir);
-	QDir localCacheDirectorySave(localCacheDir + "save");
-	QCOMPARE(localCacheDirectory.removeRecursively(), true);
-	QCOMPARE(localCacheDirectorySave.rename(localCacheDir + "save", localCacheDir), true);
-
+	// (4) move the saved local cache  backinto place and try to open the cloud repo
+	//     -> this forces a merge
+	moveDir(localCacheDir + "save", localCacheDir);
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(save_dives("./SampleDivesMinus1-merged.ssrf"), 0);
 	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
@@ -301,56 +387,60 @@ void TestGitStorage::testGitStorageCloudMerge3()
 	// edit dive notes offline
 	// edit the same dive notes in the cloud repo
 	// merge
-	clear_dive_file_data();
+
+
+	// (1) open repo, edit notes of first three dives
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
-	struct dive *dive = get_dive(0);
-	QVERIFY(dive != 0);
+	struct dive *dive;
+	QVERIFY((dive = get_dive(0)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Create multi line dive notes\nLine 2\nLine 3\nLine 4\nLine 5\nThat should be enough");
-	dive = get_dive(1);
+	QVERIFY((dive = get_dive(1)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Create multi line dive notes\nLine 2\nLine 3\nLine 4\nLine 5\nThat should be enough");
-	dive = get_dive(2);
+	QVERIFY((dive = get_dive(2)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Create multi line dive notes\nLine 2\nLine 3\nLine 4\nLine 5\nThat should be enough");
 	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	clear_dive_file_data();
 
-	QCOMPARE(parse_file(qPrintable(localCacheRepo), &dive_table, &trip_table, &dive_site_table), 0);
+	// (2) make different edits offline
+	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
-	dive = get_dive(0);
+	QVERIFY((dive = get_dive(0)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Create multi line dive notes\nDifferent line 2 and removed 3-5\n\nThat should be enough");
-	dive = get_dive(1);
+	QVERIFY((dive = get_dive(1)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Line 2\nLine 3\nLine 4\nLine 5"); // keep the middle, remove first and last");
-	dive = get_dive(2);
+	QVERIFY((dive = get_dive(2)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("single line dive notes");
-	QCOMPARE(save_dives(qPrintable(localCacheRepo)), 0);
+	git_local_only = true;
+	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
+	git_local_only = false;
 	clear_dive_file_data();
 
-	// move the local cache away
-	{ // scope for variables
-		QDir localCacheDirectory(localCacheDir);
-		QDir localCacheDirectorySave(localCacheDir + "save");
-		QCOMPARE(localCacheDirectorySave.removeRecursively(), true);
-		QCOMPARE(localCacheDirectory.rename(localCacheDir, localCacheDir + "save"), true);
-	}
-	// now we open the cloud storage repo and modify those first dive notes differently
+	// (3) simulate a second system by moving the cache away and open the cloud storage repo and modify
+	//     those first dive notes differently while online
+	moveDir(localCacheDir, localCacheDir + "save");
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	process_loaded_dives();
-	dive = get_dive(0);
+	QVERIFY((dive = get_dive(0)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Completely different dive notes\nBut also multi line");
-	dive = get_dive(1);
+	QVERIFY((dive = get_dive(1)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("single line dive notes");
-	dive = get_dive(2);
+	QVERIFY((dive = get_dive(2)) != 0);
+	free(dive->notes);
 	dive->notes = strdup("Line 2\nLine 3\nLine 4\nLine 5"); // keep the middle, remove first and last");
 	QCOMPARE(save_dives(qPrintable(cloudTestRepo)), 0);
 	clear_dive_file_data();
 
-	// now we move the saved local cache into place and try to open the cloud repo
-	// -> this forces a merge
-	QDir localCacheDirectory(localCacheDir);
-	QDir localCacheDirectorySave(localCacheDir + "save");
-	QCOMPARE(localCacheDirectory.removeRecursively(), true);
-	QCOMPARE(localCacheDirectorySave.rename(localCacheDir + "save", localCacheDir), true);
-
+	// (4) move the saved local cache back into place and open the cloud repo -> this forces a merge
+	moveDir(localCacheDir + "save", localCacheDir);
 	QCOMPARE(parse_file(qPrintable(cloudTestRepo), &dive_table, &trip_table, &dive_site_table), 0);
 	QCOMPARE(save_dives("./SampleDivesMerge3.ssrf"), 0);
 	// we are not trying to compare this to a pre-determined result... what this test
diff --git a/tests/testgitstorage.h b/tests/testgitstorage.h
index bc82c65a1..17ad4ab10 100644
--- a/tests/testgitstorage.h
+++ b/tests/testgitstorage.h
@@ -9,6 +9,7 @@ class TestGitStorage : public QObject {
 private slots:
 	void initTestCase();
 	void cleanup();
+	void cleanupTestCase();
 
 	void testGitStorageLocal_data();
 	void testGitStorageLocal();