Fix test/backupstorefix random failures caused by file upload order.

Previously, files were uploaded using bbackupd, which did it in the order that
it read the directories, which was OS-dependent. So destroying certain fixed
RaidFiles would have an unpredictable effect on the store. Now, we write a manifest
when creating the test files, and upload them in exactly that order, so that the
ID of each file will always be the same.

2 files changed, 117 insertions, 39 deletions

diff --git a/test/backupstorefix/testbackupstorefix.cpp b/test/backupstorefix/testbackupstorefix.cpp
index a8cb759c..8e0a2720 100644
--- a/test/backupstorefix/testbackupstorefix.cpp
+++ b/test/backupstorefix/testbackupstorefix.cpp
@@ -29,6 +29,7 @@
 #include "BackupStoreInfo.h"
 #include "BufferedWriteStream.h"
 #include "FileStream.h"
+#include "IOStreamGetLine.h"
 #include "RaidFileController.h"
 #include "RaidFileException.h"
 #include "RaidFileRead.h"
@@ -335,8 +336,14 @@ void test_dir_fixing()
 }
 
 int64_t fake_upload(BackupProtocolLocal& client, const std::string& file_path,
-	int64_t diff_from_id)
+	int64_t diff_from_id, int64_t container_id = BACKUPSTORE_ROOT_DIRECTORY_ID,
+	BackupStoreFilename* fn = NULL)
 {
+	if(fn == NULL)
+	{
+		fn = &fnames[0];
+	}
+
 	std::auto_ptr<IOStream> upload;
 	if(diff_from_id)
 	{
@@ -346,11 +353,13 @@ int64_t fake_upload(BackupProtocolLocal& client, const std::string& file_path,
 		std::auto_ptr<IOStream> blockIndexStream(client.ReceiveStream());
 		upload = BackupStoreFile::EncodeFileDiff(
 			file_path,
-			BACKUPSTORE_ROOT_DIRECTORY_ID, fnames[0],
-			diff_from_id, *blockIndexStream,
+			container_id,
+			*fn,
+			diff_from_id,
+			*blockIndexStream,
 			IOStream::TimeOutInfinite,
 			NULL, // DiffTimer implementation
-			0 /* not interested in the modification time */, 
+			0 /* not interested in the modification time */,
 			NULL // isCompletelyDifferent
 			);
 	}
@@ -358,18 +367,19 @@ int64_t fake_upload(BackupProtocolLocal& client, const std::string& file_path,
 	{
 		upload = BackupStoreFile::EncodeFile(
 			file_path,
-			BACKUPSTORE_ROOT_DIRECTORY_ID, fnames[0],
-			NULL, 
+			container_id,
+			*fn,
+			NULL,
 			NULL, // pLogger
 			NULL // pRunStatusProvider
 			);
 	}
 
-	return client.QueryStoreFile(BACKUPSTORE_ROOT_DIRECTORY_ID,
+	return client.QueryStoreFile(container_id,
 		1, // ModificationTime
 		2, // AttributesHash
 		diff_from_id, // DiffFromFileID
-		fnames[0], // rFilename
+		*fn, // rFilename
 		upload)->GetObjectID();
 }
 
@@ -537,17 +547,64 @@ int test(int argc, const char *argv[])
 
 	// Start the bbstored server
 	TEST_THAT_OR(StartServer(), return 1);
-	TEST_THAT_OR(StartClient(), return 1);
 
-	// Wait 4 more seconds for the files to be old enough
-	// to upload
-	::safe_sleep(4);
+	// Instead of starting a client, read the file listing file created by
+	// testbackupstorefix.pl and upload them in the correct order, so that the object
+	// IDs will not vary depending on the order in which readdir() returns entries.
+	{
+		FileStream listing("testfiles/file-listing.txt", O_RDONLY);
+		IOStreamGetLine getline(listing);
+		std::map<std::string, int64_t> dirname_to_id;
+		std::string line;
+		BackupProtocolLocal2 client(0x01234567, "test", accountRootDir,
+			discSetNum, false);
+
+		for(getline.GetLine(line, true); line != ""; getline.GetLine(line, true))
+		{
+			std::string full_path = line;
+			ASSERT(StartsWith("testfiles/TestDir1/", full_path));
+
+			bool is_dir = (full_path[full_path.size() - 1] == '/');
+			if(is_dir)
+			{
+				full_path = full_path.substr(0, full_path.size() - 1);
+			}
 
-	// Upload files to create a nice store directory
-	::sync_and_wait();
+			std::string::size_type last_slash = full_path.rfind('/');
+			int64_t container_id;
+			std::string filename;
 
-	// Stop bbackupd
-	TEST_THAT_OR(StopClient(), return 1);
+			if(full_path == "testfiles/TestDir1")
+			{
+				container_id = BACKUPSTORE_ROOT_DIRECTORY_ID;
+				filename = "Test1";
+			}
+			else
+			{
+				std::string containing_dir =
+					full_path.substr(0, last_slash);
+				container_id = dirname_to_id[containing_dir];
+				filename = full_path.substr(last_slash + 1);
+			}
+
+			BackupStoreFilenameClear fn(filename);
+			if(is_dir)
+			{
+				std::auto_ptr<IOStream> attr_stream(
+					new CollectInBufferStream);
+				((CollectInBufferStream &)
+					*attr_stream).SetForReading();
+
+				dirname_to_id[full_path] = client.QueryCreateDirectory(
+					container_id, 0, // AttributesModTime
+					fn, attr_stream)->GetObjectID();
+			}
+			else
+			{
+				fake_upload(client, line, 0, container_id, &fn);
+			}
+		}
+	}
 
 	// Check that we're starting off with the right numbers of files and blocks.
 	// Otherwise the test that check the counts after breaking things will fail
@@ -839,8 +896,11 @@ int test(int argc, const char *argv[])
 		}
 		SaveDirectory("Test1/cannes/ict/peep", dir);
 	}
-	// Delete a directory
+
+	// Delete a directory. The checker should be able to reconstruct it using the
+	// ContainerID of the contained files.
 	DeleteObject("Test1/pass/cacted/ming");
+
 	// Delete a file
 	DeleteObject("Test1/cannes/ict/scely");
 
@@ -848,7 +908,7 @@ int test(int argc, const char *argv[])
 	// spotting errors! But asserting an exact number will help us catch
 	// changes in checker behaviour, so it's not a bad thing to test.
 
-	// The 12 errors are:
+	// The 12 errors that we currently expect are:
 	// ERROR:   Directory ID 0xb references object 0x3e which does not exist.
 	// ERROR:   Removing directory entry 0x3e from directory 0xb
 	// ERROR:   Directory ID 0xc had invalid entries, fixed
@@ -876,7 +936,7 @@ int test(int argc, const char *argv[])
 	}
 
 	// Check everything is as it should be
-	TEST_THAT(::system(PERL_EXECUTABLE 
+	TEST_THAT(::system(PERL_EXECUTABLE
 		" testfiles/testbackupstorefix.pl check 2") == 0);
 	{
 		BackupStoreDirectory dir;
@@ -931,10 +991,12 @@ int test(int argc, const char *argv[])
 		dir2.AddEntry(*en);
 		SaveDirectory("Test1/divel/torsines/cruishery", dir2);
 	}
+
 	// Fix it
 	RUN_CHECK
+
 	// Check everything is as it should be
-	TEST_THAT(::system(PERL_EXECUTABLE 
+	TEST_THAT(::system(PERL_EXECUTABLE
 		" testfiles/testbackupstorefix.pl check 3") == 0);
 	{
 		BackupStoreDirectory dir;
@@ -946,10 +1008,12 @@ int test(int argc, const char *argv[])
 	BOX_INFO("  === Orphan files and dirs without being recoverable");
 	DeleteObject("Test1/dir1");
 	DeleteObject("Test1/dir1/dir2");
+
 	// Fix it
 	RUN_CHECK
+
 	// Check everything is where it is predicted to be
-	TEST_THAT(::system(PERL_EXECUTABLE 
+	TEST_THAT(::system(PERL_EXECUTABLE
 		" testfiles/testbackupstorefix.pl check 4") == 0);
 
 	// ------------------------------------------------------------------------------------------------
diff --git a/test/backupstorefix/testfiles/testbackupstorefix.pl.in b/test/backupstorefix/testfiles/testbackupstorefix.pl.in
index 6914a0f1..fc807155 100755
--- a/test/backupstorefix/testfiles/testbackupstorefix.pl.in
+++ b/test/backupstorefix/testfiles/testbackupstorefix.pl.in
@@ -34,16 +34,26 @@ my @check_move = (
 
 if($ARGV[0] eq 'init')
 {
+	open(my $fh, ">>", "testfiles/file-listing.txt")
+		or die "cannot open testfiles/file-listing.txt: $!";
 	# create the initial tree of words
-	make_dir('testfiles/TestDir1', 0, 4, 0);
-	
+	make_dir($fh, 'testfiles/TestDir1', 0, 4, 0);
+
 	# add some useful extra bits to it
-	mkdir('testfiles/TestDir1/dir-no-members', 0755);
-	mkdir('testfiles/TestDir1/dir1', 0755);
-	mkdir('testfiles/TestDir1/dir1/dir2', 0755);
-	mkdir('testfiles/TestDir1/dir1/dir2/dir3', 0755);
-	make_file('testfiles/TestDir1/dir1/dir2/file1');
-	make_file('testfiles/TestDir1/dir1/dir2/dir3/file2');
+	foreach my $subdir (
+		'testfiles/TestDir1/dir-no-members',
+		'testfiles/TestDir1/dir1',
+		'testfiles/TestDir1/dir1/dir2',
+		'testfiles/TestDir1/dir1/dir2/dir3',
+	)
+	{
+		mkdir($subdir, 0755);
+		print $fh "$subdir/\n";
+	}
+	make_file($fh, 'testfiles/TestDir1/dir1/dir2/file1');
+	make_file($fh, 'testfiles/TestDir1/dir1/dir2/dir3/file2');
+
+	close $fh;
 }
 elsif($ARGV[0] eq 'check')
 {
@@ -172,20 +182,19 @@ else
 
 sub make_dir
 {
-	my ($dir,$start,$quantity,$level) = @_;
-
+	my ($fh,$dir,$start,$quantity,$level) = @_;
 	return $start if $level >= 4;
 
 	mkdir $dir,0755;
-	
+	print $fh "$dir/\n";
 	return $start if $start > $#words;
-	
+
 	while($start <= $#words && $quantity > 0)
 	{
 		my $subdirs = length($words[$start]) - 2;
 		$subdirs = 2 if $subdirs > 2;
 		my $entries = $subdirs + 1;
-		
+
 		for(0 .. ($entries - 1))
 		{
 			my $w = $words[$start + $_];
@@ -198,26 +207,31 @@ sub make_dir
 			}
 			print FL "\n";
 			close FL;
+
+			print $fh "$dir/$w\n";
 		}
+
 		$start += $entries;
 		my $w = $words[$start + $_];
-		$start = make_dir("$dir/$w", $start + 1, $subdirs, $level + 1);
-	
+		$start = make_dir($fh, "$dir/$w", $start + 1, $subdirs, $level + 1);
+
 		$quantity--;
 	}
-	
+
 	return $start;
 }
 
 sub make_file
 {
-	my ($fn) = @_;
-	
+	my ($fh, $fn) = @_;
+
 	open FL,'>'.$fn or die "can't open $fn for writing";
 	for(0 .. 255)
 	{
 		print FL $fn
 	}
 	close FL;
+
+	print $fh "$fn\n";
 }