Skip to content
This repository has been archived by the owner on Oct 7, 2022. It is now read-only.

Fix tests on windows and add UB warning for YAML DeSer #92

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ features = ["yaml_enc"]

You can now use `rustbreak::deser::Yaml` as deserialization struct.

🔥🔥
__Warning__: Using this deserializer can trigger *Undefined Behaviour (UB)*. It
is *strongly* recommended to *not* use this deserializer until these issues are
fixed. See [tracking issue #87] for more details.
🔥🔥

### Ron

If you would like to use [`ron`](https://github.com/ron-rs/ron) you need to
Expand Down Expand Up @@ -143,3 +149,4 @@ You can now use `rustbreak::deser::Bincode` as deserialization struct.

[doc]:http://neikos.me/rustbreak/rustbreak/index.html
[Daybreak]:https://propublica.github.io/daybreak/
[tracking issue #87]: https://github.com/TheNeikos/rustbreak/issues/87
27 changes: 18 additions & 9 deletions src/backend/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,25 +90,29 @@ impl Backend for PathBackend {
mod tests {
use super::{Backend, PathBackend};
use std::io::Write;
use tempfile::NamedTempFile;
use std::path::Path;
use tempfile::{tempdir, NamedTempFile};

#[test]
#[cfg_attr(miri, ignore)]
fn test_path_backend_existing() {
let file = NamedTempFile::new().expect("could not create temporary file");
let (mut backend, existed) = PathBackend::from_path_or_create(file.path().to_owned())
.expect("could not create backend");
let tmppath = file.into_temp_path();
let path: &Path = tmppath.as_ref();
let (mut backend, existed) =
PathBackend::from_path_or_create(path.to_owned()).expect("could not create backend");
assert!(existed);
let data = [4, 5, 1, 6, 8, 1];

backend.put_data(&data).expect("could not put data");
assert_eq!(backend.get_data().expect("could not get data"), data);
tmppath.close().expect("could not delete temp file");
}

#[test]
#[cfg_attr(miri, ignore)]
fn test_path_backend_new() {
let dir = tempfile::tempdir().expect("could not create temporary directory");
let dir = tempdir().expect("could not create temporary directory");
let mut file_path = dir.path().to_owned();
file_path.push("rustbreak_path_db.db");
let (mut backend, existed) =
Expand All @@ -125,18 +129,20 @@ mod tests {
#[cfg_attr(miri, ignore)]
fn test_path_backend_nofail() {
let file = NamedTempFile::new().expect("could not create temporary file");
let file_path = file.path().to_owned();
let mut backend = PathBackend::from_path_or_fail(file_path).expect("should not fail");
let tmppath = file.into_temp_path();
let path: &Path = tmppath.as_ref();
let mut backend = PathBackend::from_path_or_fail(path.to_owned()).expect("should not fail");
let data = [4, 5, 1, 6, 8, 1];

backend.put_data(&data).expect("could not put data");
assert_eq!(backend.get_data().expect("could not get data"), data);
tmppath.close().expect("could not delete temp file");
}

#[test]
#[cfg_attr(miri, ignore)]
fn test_path_backend_fail_notfound() {
let dir = tempfile::tempdir().expect("could not create temporary directory");
let dir = tempdir().expect("could not create temporary directory");
let mut file_path = dir.path().to_owned();
file_path.push("rustbreak_path_db.db");
let err =
Expand All @@ -154,21 +160,24 @@ mod tests {
#[cfg_attr(miri, ignore)]
fn test_path_backend_create_and_existing_nocall() {
let file = NamedTempFile::new().expect("could not create temporary file");
let mut backend = PathBackend::from_path_or_create_and(file.path().to_owned(), |_| {
let tmppath = file.into_temp_path();
let path: &Path = tmppath.as_ref();
let mut backend = PathBackend::from_path_or_create_and(path.to_owned(), |_| {
panic!("Closure called but file already existed");
})
.expect("could not create backend");
let data = [4, 5, 1, 6, 8, 1];

backend.put_data(&data).expect("could not put data");
assert_eq!(backend.get_data().expect("could not get data"), data);
tmppath.close().expect("could not delete temp file");
}

// If the file does not yet exist, the closure should be called.
#[test]
#[cfg_attr(miri, ignore)]
fn test_path_backend_create_and_new() {
let dir = tempfile::tempdir().expect("could not create temporary directory");
let dir = tempdir().expect("could not create temporary directory");
let mut file_path = dir.path().to_owned();
file_path.push("rustbreak_path_db.db");
let mut backend = PathBackend::from_path_or_create_and(file_path, |f| {
Expand Down
20 changes: 16 additions & 4 deletions src/deser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ pub use self::bincode::Bincode;
/// extern crate thiserror;
/// extern crate serde;
/// #[macro_use]
///
/// use serde::de::Deserialize;
/// use serde::Serialize;
/// use std::io::Read;
Expand Down Expand Up @@ -69,7 +68,8 @@ pub use self::bincode::Bincode;
/// fn main() {}
/// ```
///
/// **Important**: You can only return custom errors if the `other_errors` feature is enabled
/// **Important**: You can only return custom errors if the `other_errors`
/// feature is enabled
pub trait DeSerializer<T: Serialize + DeserializeOwned>:
std::default::Default + Send + Sync + Clone
{
Expand All @@ -93,7 +93,7 @@ mod ron {
use crate::deser::DeSerializer;
use crate::error;

/// The Struct that allows you to use `ron` the Rusty Object Notation.
/// The Struct that allows you to use `ron`, the Rusty Object Notation.
#[derive(Debug, Default, Clone)]
pub struct Ron;

Expand All @@ -118,7 +118,19 @@ mod yaml {
use crate::deser::DeSerializer;
use crate::error;

/// The struct that allows you to use yaml.
/// The struct that allows you to use yaml. 🔥 *do not use* 🔥
///
/// 🔥🔥 __Warning__: Using this [`DeSerializer`] can trigger *Undefined
/// Behaviour (UB)*. 🔥🔥 It is *strongly* recommended to *not* use this
/// [`DeSerializer`] until these issues are fixed. The UB is triggered
/// in a transitive dependency (namely [`linked_hash_map`]) of Rustbreak.
/// There is nothing the Rustbreak devs can do about this.
/// The UB is real and reachable. It triggered by the Rustbreak test suite,
/// and detected by `miri`. See the [tracking issue #87] for more details.
/// 🔥🔥 __DO NOT USE__ 🔥🔥
///
/// [`linked_hash_map`]: https://github.com/contain-rs/linked-hash-map
/// [tracking issue #87]: https://github.com/TheNeikos/rustbreak/issues/87
#[derive(Debug, Default, Clone)]
pub struct Yaml;

Expand Down
16 changes: 8 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,11 +328,7 @@ where
/// # extern crate rustbreak;
/// # extern crate serde;
/// # extern crate tempfile;
/// use rustbreak::{
/// deser::Ron,
/// error::RustbreakError,
/// FileDatabase,
/// };
/// use rustbreak::{deser::Ron, error::RustbreakError, FileDatabase};
///
/// #[derive(Debug, Serialize, Deserialize, Clone)]
/// struct Data {
Expand Down Expand Up @@ -1010,6 +1006,7 @@ where
mod tests {
use super::*;
use std::collections::HashMap;
use std::path::Path;
use tempfile::NamedTempFile;

type TestData = HashMap<usize, String>;
Expand Down Expand Up @@ -1448,16 +1445,19 @@ mod tests {
#[cfg_attr(miri, ignore)]
fn pathdb_from_path_existing() {
let file = NamedTempFile::new().expect("could not create temporary file");
let path = file.path().to_owned();
let tmppath = file.into_temp_path();
let path: &Path = tmppath.as_ref();
// initialise the file
let db = TestDb::<PathBackend>::create_at_path(path.clone(), test_data())
let db = TestDb::<PathBackend>::create_at_path(path.to_owned(), test_data())
.expect("could not create db");
db.save().expect("could not save db");
drop(db);
// test that loading now works
let db = TestDb::<PathBackend>::load_from_path(path).expect("could not load");
let db = TestDb::<PathBackend>::load_from_path(path.to_owned()).expect("could not load");
let readlock = db.borrow_data().expect("Rustbreak readlock error");
assert_eq!(test_data(), *readlock);

tmppath.close().expect("could not delete temp file");
}

#[test]
Expand Down
27 changes: 21 additions & 6 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,12 @@ fn create_mmapdb_with_size<S: DeSerializer<Data> + Debug>(size: usize) -> MmapDa
MmapDatabase::mmap_with_size(Data::default(), size).expect("could not create database")
}

fn create_pathdb<S: DeSerializer<Data> + Debug>() -> PathDatabase<Data, S> {
fn create_pathdb<S: DeSerializer<Data> + Debug>() -> (PathDatabase<Data, S>, tempfile::TempPath) {
let file = tempfile::NamedTempFile::new().expect("could not create temporary file");
PathDatabase::create_at_path(file.path().to_owned(), Data::default())
.expect("could not create database")
let db = PathDatabase::create_at_path(file.path().to_owned(), Data::default())
.expect("could not create database");

(db, file.into_temp_path())
}

macro_rules! test_basic_save_load {
Expand All @@ -151,6 +153,19 @@ macro_rules! test_basic_save_load {
test_convert_data(db);
}
};
($name:ident, $db:expr, $enc:ty, setupenv=true) => {
#[test]
#[cfg_attr(miri, ignore)]
fn $name() {
let env = $db;
let db: Database<Data, _, $enc> = env.0;
test_basic_save_load(&db);
test_put_data(&db);
test_multi_borrow(&db);
test_convert_data(db);
env.1.close().expect("Could not close environment");
}
};
}

test_basic_save_load!(file_ron, create_filedb(), Ron);
Expand All @@ -173,6 +188,6 @@ test_basic_save_load!(mmapsize_ron, create_mmapdb_with_size(10), Ron);
test_basic_save_load!(mmapsize_yaml, create_mmapdb_with_size(10), Yaml);
test_basic_save_load!(mmapsize_bincode, create_mmapdb_with_size(10), Bincode);

test_basic_save_load!(path_ron, create_pathdb(), Ron);
test_basic_save_load!(path_yaml, create_pathdb(), Yaml);
test_basic_save_load!(path_bincode, create_pathdb(), Bincode);
test_basic_save_load!(path_ron, create_pathdb(), Ron, setupenv = true);
test_basic_save_load!(path_yaml, create_pathdb(), Yaml, setupenv = true);
test_basic_save_load!(path_bincode, create_pathdb(), Bincode, setupenv = true);