From f71953dcd99fdd42f8bac460da7d39b00ea27588 Mon Sep 17 00:00:00 2001 From: Jesse Rusak Date: Tue, 24 Mar 2020 04:36:39 -0400 Subject: [PATCH] Make DBPath use tempfile. (#394) --- Cargo.toml | 1 + tests/test_backup.rs | 13 +++++++------ tests/test_iterator.rs | 8 ++------ tests/test_rocksdb_options.rs | 2 +- tests/util/mod.rs | 33 +++++++++++++++++---------------- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3e7ea5e..9e02faa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,3 +29,4 @@ librocksdb-sys = { path = "librocksdb-sys", version = "6.4.6" } [dev-dependencies] trybuild = "1.0.21" +tempfile = "3.1.0" diff --git a/tests/test_backup.rs b/tests/test_backup.rs index cf4e91e..385db02 100644 --- a/tests/test_backup.rs +++ b/tests/test_backup.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +mod util; +use crate::util::DBPath; + use rocksdb::{ backup::{BackupEngine, BackupEngineOptions, RestoreOptions}, Options, DB, @@ -20,12 +23,12 @@ use rocksdb::{ #[test] fn backup_restore() { // create backup - let path = "_rust_rocksdb_backup_test"; - let restore_path = "_rust_rocksdb_restore_from_backup_path"; + let path = DBPath::new("backup_test"); + let restore_path = DBPath::new("restore_from_backup_path"); let mut opts = Options::default(); opts.create_if_missing(true); { - let db = DB::open(&opts, path).unwrap(); + let db = DB::open(&opts, &path).unwrap(); assert!(db.put(b"k1", b"v1111").is_ok()); let value = db.get(b"k1"); assert_eq!(value.unwrap().unwrap(), b"v1111"); @@ -44,11 +47,9 @@ fn backup_restore() { ); assert!(restore_status.is_ok()); - let db_restore = DB::open_default(restore_path).unwrap(); + let db_restore = DB::open_default(&restore_path).unwrap(); let value = db_restore.get(b"k1"); assert_eq!(value.unwrap().unwrap(), b"v1111"); } } - assert!(DB::destroy(&opts, restore_path).is_ok()); - assert!(DB::destroy(&opts, path).is_ok()); } diff --git a/tests/test_iterator.rs b/tests/test_iterator.rs index de417a3..a329637 100644 --- a/tests/test_iterator.rs +++ b/tests/test_iterator.rs @@ -253,7 +253,7 @@ fn test_prefix_iterator_uses_full_prefix() { #[test] fn test_full_iterator() { - let path = "_rust_rocksdb_fulliteratortest"; + let path = DBPath::new("fulliteratortest"); { let a1: Box<[u8]> = key(b"aaa1"); let a2: Box<[u8]> = key(b"aaa2"); @@ -273,7 +273,7 @@ fn test_full_iterator() { opts.set_allow_concurrent_memtable_write(false); opts.set_memtable_factory(factory); - let db = DB::open(&opts, path).unwrap(); + let db = DB::open(&opts, &path).unwrap(); assert!(db.put(&*a1, &*a1).is_ok()); assert!(db.put(&*a2, &*a2).is_ok()); @@ -295,8 +295,6 @@ fn test_full_iterator() { let a_iterator = db.full_iterator(IteratorMode::Start); assert_eq!(a_iterator.collect::>(), expected) } - let opts = Options::default(); - assert!(DB::destroy(&opts, path).is_ok()); } fn custom_iter<'a>(db: &'a DB) -> impl Iterator + 'a { @@ -313,8 +311,6 @@ fn test_custom_iterator() { let db = DB::open(&opts, &path).unwrap(); let _data = custom_iter(&db).collect::>(); } - let opts = Options::default(); - assert!(DB::destroy(&opts, path).is_ok()); } #[test] diff --git a/tests/test_rocksdb_options.rs b/tests/test_rocksdb_options.rs index 5c42879..8251828 100644 --- a/tests/test_rocksdb_options.rs +++ b/tests/test_rocksdb_options.rs @@ -69,7 +69,7 @@ fn test_block_based_options() { let _db = DB::open(&opts, &n).unwrap(); // read the setting from the LOG file - let mut rocksdb_log = fs::File::open(format!("{}/LOG", n.as_ref().to_str().unwrap())) + let mut rocksdb_log = fs::File::open(format!("{}/LOG", (&n).as_ref().to_str().unwrap())) .expect("rocksdb creates a LOG file"); let mut settings = String::new(); rocksdb_log.read_to_string(&mut settings).unwrap(); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index abc4d8a..ffec306 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -1,39 +1,40 @@ use std::path::{Path, PathBuf}; -use std::time::{SystemTime, UNIX_EPOCH}; +use tempfile; use rocksdb::{Options, DB}; -/// Ensures that DB::Destroy is called for this database when DBPath is dropped. +/// Temporary database path which calls DB::Destroy when DBPath is dropped. pub struct DBPath { + #[allow(dead_code)] + dir: tempfile::TempDir, // kept for cleaning up during drop path: PathBuf, } impl DBPath { - /// Suffixes the given `prefix` with a timestamp to ensure that subsequent test runs don't reuse - /// an old database in case of panics prior to Drop being called. + /// Produces a fresh (non-existent) temporary path which will be DB::destroy'ed automatically. pub fn new(prefix: &str) -> DBPath { - let current_time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); - let path = format!( - "{}.{}.{}", - prefix, - current_time.as_secs(), - current_time.subsec_nanos() - ); + let dir = tempfile::Builder::new() + .prefix(prefix) + .tempdir() + .expect("Failed to create temporary path for db."); + let path = dir.path().join("db"); - DBPath { - path: PathBuf::from(path), - } + DBPath { dir, path } } } impl Drop for DBPath { fn drop(&mut self) { let opts = Options::default(); - DB::destroy(&opts, &self.path).unwrap(); + DB::destroy(&opts, &self.path).expect("Failed to destroy temporary DB"); } } -impl AsRef for DBPath { +/// Convert a DBPath ref to a Path ref. +/// We don't implement this for DBPath values because we want them to +/// exist until the end of their scope, not get passed in to functions and +/// dropped early. +impl AsRef for &DBPath { fn as_ref(&self) -> &Path { &self.path }