From 7a31960ee94bfa41b73889104fa2e36adb704857 Mon Sep 17 00:00:00 2001 From: Amit Arya Date: Tue, 8 Sep 2015 14:23:42 -0700 Subject: [PATCH] Tests for ManifestDumpCommand and ListColumnFamiliesCommand Summary: Added tests for two LDBCommands namely i) ManifestDumpCommand and ii) ListColumnFamiliesCommand. + Minor fix in the sscanf formatter (along relace C cast with C++ cast) + replacing localtime with localtime_r which is thread safe. Test Plan: make all && ./tools/ldb_test.py Reviewers: anthony, igor, IslamAbdelRahman, kradhakrishnan, lgalanis, rven, sdong Reviewed By: sdong Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D45819 --- tools/ldb_test.py | 63 ++++++++++++++++++++++++++++++++++++++++++++--- util/ldb_cmd.cc | 13 +++++----- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/tools/ldb_test.py b/tools/ldb_test.py index 0a4326960..bcf362404 100644 --- a/tools/ldb_test.py +++ b/tools/ldb_test.py @@ -1,10 +1,12 @@ import os +import glob import os.path import shutil import subprocess import time import unittest import tempfile +import re def my_check_output(*popenargs, **kwargs): """ @@ -43,7 +45,8 @@ class LDBTestCase(unittest.TestCase): def dbParam(self, dbName): return "--db=%s" % os.path.join(self.TMP_DIR, dbName) - def assertRunOKFull(self, params, expectedOutput, unexpected=False): + def assertRunOKFull(self, params, expectedOutput, unexpected=False, + isPattern=False): """ All command-line params must be specified. Allows full flexibility in testing; for example: missing db param. @@ -53,9 +56,16 @@ class LDBTestCase(unittest.TestCase): output = my_check_output("./ldb %s |grep -v \"Created bg thread\"" % params, shell=True) if not unexpected: - self.assertEqual(output.strip(), expectedOutput.strip()) + if isPattern: + self.assertNotEqual(expectedOutput.search(output.strip()), + None) + else: + self.assertEqual(output.strip(), expectedOutput.strip()) else: - self.assertNotEqual(output.strip(), expectedOutput.strip()) + if isPattern: + self.assertEqual(expectedOutput.search(output.strip()), None) + else: + self.assertNotEqual(output.strip(), expectedOutput.strip()) def assertRunFAILFull(self, params): """ @@ -395,5 +405,52 @@ class LDBTestCase(unittest.TestCase): dumpFilePath = os.path.join(self.TMP_DIR, "dump2") self.assertTrue(self.dumpLiveFiles("--db=%s" % dbPath, dumpFilePath)) + def getManifests(self, directory): + return glob.glob(directory + "/MANIFEST-*") + + def copyManifests(self, src, dest): + return 0 == run_err_null("cp " + src + " " + dest) + + def testManifestDump(self): + print "Running testManifestDump..." + dbPath = os.path.join(self.TMP_DIR, self.DB_NAME) + self.assertRunOK("put 1 1 --create_if_missing", "OK") + self.assertRunOK("put 2 2", "OK") + self.assertRunOK("put 3 3", "OK") + # Pattern to expect from manifest_dump. + num = "[0-9]+" + st = ".*" + subpat = st + " @ " + num + ": " + num + regex = num + ":" + num + "\[" + subpat + ".." + subpat + "\]" + expected_pattern = re.compile(regex) + cmd = "manifest_dump --db=%s" + manifest_files = self.getManifests(dbPath) + self.assertTrue(len(manifest_files) == 1) + # Test with the default manifest file in dbPath. + self.assertRunOKFull(cmd % dbPath, expected_pattern, + unexpected=False, isPattern=True) + self.copyManifests(manifest_files[0], manifest_files[0] + "1") + manifest_files = self.getManifests(dbPath) + self.assertTrue(len(manifest_files) == 2) + # Test with multiple manifest files in dbPath. + self.assertRunFAILFull(cmd % dbPath) + # Running it with the copy we just created should pass. + self.assertRunOKFull((cmd + " --path=%s") + % (dbPath, manifest_files[1]), + expected_pattern, unexpected=False, + isPattern=True) + + def testListColumnFamilies(self): + print "Running testListColumnFamilies..." + dbPath = os.path.join(self.TMP_DIR, self.DB_NAME) + self.assertRunOK("put x1 y1 --create_if_missing", "OK") + cmd = "list_column_families %s | grep -v \"Column families\"" + # Test on valid dbPath. + self.assertRunOKFull(cmd % dbPath, "{default}") + # Test on empty path. + self.assertRunFAILFull(cmd % "") + + + if __name__ == "__main__": unittest.main() diff --git a/util/ldb_cmd.cc b/util/ldb_cmd.cc index c17690e05..33d9a3111 100644 --- a/util/ldb_cmd.cc +++ b/util/ldb_cmd.cc @@ -608,12 +608,9 @@ void ManifestDumpCommand::DoCommand() { struct dirent* entry; while ((entry = readdir(d.get())) != nullptr) { unsigned int match; - unsigned long long num; - if (sscanf(entry->d_name, - "MANIFEST-%ln%ln", - (unsigned long*)&num, - (unsigned long*)&match) - && match == strlen(entry->d_name)) { + uint64_t num; + if (sscanf(entry->d_name, "MANIFEST-%lu%n", &num, &match) && + match == strlen(entry->d_name)) { if (!found) { manifestfile = db_path_ + "/" + std::string(entry->d_name); found = true; @@ -686,7 +683,9 @@ namespace { string ReadableTime(int unixtime) { char time_buffer [80]; time_t rawtime = unixtime; - struct tm * timeinfo = localtime(&rawtime); + struct tm tInfo; + struct tm* timeinfo = localtime_r(&rawtime, &tInfo); + assert(timeinfo == &tInfo); strftime(time_buffer, 80, "%c", timeinfo); return string(time_buffer); }