Skip to content

Added some more vmm unit tests #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
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
3 changes: 2 additions & 1 deletion api_server/src/request/sync/drive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl DriveDescription {
}
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum DriveError {
RootBlockDeviceAlreadyAdded,
InvalidBlockDevicePath,
Expand Down Expand Up @@ -76,6 +76,7 @@ impl GenerateResponse for DriveError {
}
}

#[derive(PartialEq)]
pub enum PutDriveOutcome {
Created,
Updated,
Expand Down
1 change: 1 addition & 0 deletions api_server/src/request/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl fmt::Debug for SyncRequest {
// TODO: we should move toward having both the ok status and various possible sync request errors
// in this file, because there are many common sync outcomes.

#[derive(PartialEq)]
pub enum OkStatus {
Created,
Updated,
Expand Down
3 changes: 3 additions & 0 deletions vmm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ net_util = { path = "../net_util"}
sys_util = { path = "../sys_util" }
x86_64 = { path = "../x86_64" }

[dev-dependencies]
tempfile = ">=3.0.2"

168 changes: 74 additions & 94 deletions vmm/src/device_config/drive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,10 @@ impl BlockDeviceConfigs {

#[cfg(test)]
mod tests {
extern crate tempfile;

use super::*;
use api_server::request::sync::{DeviceState, DrivePermissions};
use std::fs::{self, File};

// Helper function for creating a dummy file
// The filename has to be unique among all tests because the tests are run on many threads
fn create_dummy_path(filename: String) -> PathBuf {
let _file = File::create(filename.clone());
return PathBuf::from(filename);
}

// Helper function for deleting a dummy file
fn delete_dummy_path(filename: String) {
let _rs = fs::remove_file(filename);
}

#[test]
fn test_create_block_devices_configs() {
Expand All @@ -180,10 +169,9 @@ mod tests {

#[test]
fn test_add_non_root_block_device() {
let dummy_filename = String::from("test_add_non_root_block_device");
let dummy_path = create_dummy_path(dummy_filename.clone());
let dummy_file = tempfile::NamedTempFile::new().unwrap();
let dummy_path = dummy_file.path().to_path_buf();
let dummy_id = String::from("1");

let dummy_block_device = BlockDeviceConfig {
path_on_host: dummy_path.clone(),
is_root_device: false,
Expand All @@ -193,25 +181,25 @@ mod tests {
};

let mut block_devices_configs = BlockDeviceConfigs::new();
let ret = block_devices_configs.add(dummy_block_device.clone());
// clean up before the assert!s to make sure the temp files are deleted
delete_dummy_path(dummy_filename);
assert!(ret.is_ok());
assert!(
block_devices_configs
.add(dummy_block_device.clone())
.is_ok()
);

assert_eq!(block_devices_configs.has_root_block, false);
assert_eq!(block_devices_configs.config_list.len(), 1);

let dev_config = block_devices_configs.config_list.iter().next().unwrap();
assert_eq!(dev_config, &dummy_block_device);
assert!(block_devices_configs.contains_drive_path(dummy_path));
assert!(!block_devices_configs.contains_drive_path(PathBuf::from("/foo/bar")));
assert!(block_devices_configs.contains_drive_id(dummy_id.clone()));
assert!(!block_devices_configs.contains_drive_id(String::from("foo")));
assert!(block_devices_configs.contains_drive_id(dummy_id));
}

#[test]
fn test_add_one_root_block_device() {
let dummy_filename = String::from("test_add_one_root_block_device");
let dummy_path = create_dummy_path(dummy_filename.clone());
let dummy_file = tempfile::NamedTempFile::new().unwrap();
let dummy_path = dummy_file.path().to_path_buf();

let dummy_block_device = BlockDeviceConfig {
path_on_host: dummy_path,
Expand All @@ -220,10 +208,13 @@ mod tests {
drive_id: String::from("1"),
rate_limiter: None,
};

let mut block_devices_configs = BlockDeviceConfigs::new();
let ret = block_devices_configs.add(dummy_block_device.clone());
delete_dummy_path(dummy_filename);
assert!(ret.is_ok());
assert!(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This asserts on is_ok (and not on _eq(Ok())), please change the previous one to do the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do.

block_devices_configs
.add(dummy_block_device.clone())
.is_ok()
);

assert_eq!(block_devices_configs.has_root_block, true);
assert_eq!(block_devices_configs.config_list.len(), 1);
Expand All @@ -234,8 +225,8 @@ mod tests {

#[test]
fn test_add_two_root_block_devices_configs() {
let dummy_filename_1 = String::from("test_add_two_root_block_devices_configs_1");
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
let dummy_file_1 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_1 = dummy_file_1.path().to_path_buf();
let root_block_device_1 = BlockDeviceConfig {
path_on_host: dummy_path_1,
is_root_device: true,
Expand All @@ -244,8 +235,8 @@ mod tests {
rate_limiter: None,
};

let dummy_filename_2 = String::from("test_add_two_root_block_devices_configs_2");
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
let dummy_file_2 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_2 = dummy_file_2.path().to_path_buf();
let root_block_device_2 = BlockDeviceConfig {
path_on_host: dummy_path_2,
is_root_device: true,
Expand All @@ -255,25 +246,18 @@ mod tests {
};

let mut block_devices_configs = BlockDeviceConfigs::new();
let ret = block_devices_configs.add(root_block_device_1);
let actual_error = format!(
"{:?}",
block_devices_configs.add(root_block_device_2).unwrap_err()
assert!(block_devices_configs.add(root_block_device_1).is_ok());
assert_eq!(
block_devices_configs.add(root_block_device_2).unwrap_err(),
DriveError::RootBlockDeviceAlreadyAdded
);
let expected_error = format!("{:?}", DriveError::RootBlockDeviceAlreadyAdded);

delete_dummy_path(dummy_filename_1);
delete_dummy_path(dummy_filename_2);

assert!(ret.is_ok());
assert_eq!(expected_error, actual_error);
}

#[test]
/// Test BlockDevicesConfigs::add when you first add the root device and then the other devices
fn test_add_root_block_device_first() {
let dummy_filename_1 = String::from("test_add_root_block_device_first_1");
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
let dummy_file_1 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_1 = dummy_file_1.path().to_path_buf();
let root_block_device = BlockDeviceConfig {
path_on_host: dummy_path_1,
is_root_device: true,
Expand All @@ -282,8 +266,8 @@ mod tests {
rate_limiter: None,
};

let dummy_filename_2 = String::from("test_add_root_block_device_first_2");
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
let dummy_file_2 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_2 = dummy_file_2.path().to_path_buf();
let dummy_block_device_2 = BlockDeviceConfig {
path_on_host: dummy_path_2,
is_root_device: false,
Expand All @@ -292,8 +276,8 @@ mod tests {
rate_limiter: None,
};

let dummy_filename_3 = String::from("test_add_root_block_device_first_3");
let dummy_path_3 = create_dummy_path(dummy_filename_3.clone());
let dummy_file_3 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_3 = dummy_file_3.path().to_path_buf();
let dummy_block_device_3 = BlockDeviceConfig {
path_on_host: dummy_path_3,
is_root_device: false,
Expand All @@ -303,17 +287,18 @@ mod tests {
};

let mut block_devices_configs = BlockDeviceConfigs::new();
let ret1 = block_devices_configs.add(root_block_device.clone());
let ret2 = block_devices_configs.add(dummy_block_device_2.clone());
let ret3 = block_devices_configs.add(dummy_block_device_3.clone());

delete_dummy_path(dummy_filename_1);
delete_dummy_path(dummy_filename_2);
delete_dummy_path(dummy_filename_3);

assert!(ret1.is_ok());
assert!(ret2.is_ok());
assert!(ret3.is_ok());
assert!(block_devices_configs.add(root_block_device.clone()).is_ok());
assert!(
block_devices_configs
.add(dummy_block_device_2.clone())
.is_ok()
);
assert!(
block_devices_configs
.add(dummy_block_device_3.clone())
.is_ok()
);

assert_eq!(block_devices_configs.has_root_block_device(), true);
assert_eq!(block_devices_configs.config_list.len(), 3);
Expand All @@ -327,8 +312,8 @@ mod tests {
#[test]
/// Test BlockDevicesConfigs::add when you add other devices first and then the root device
fn test_root_block_device_add_last() {
let dummy_filename_1 = String::from("test_root_block_device_add_last_1");
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
let dummy_file_1 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_1 = dummy_file_1.path().to_path_buf();
let root_block_device = BlockDeviceConfig {
path_on_host: dummy_path_1,
is_root_device: true,
Expand All @@ -337,8 +322,8 @@ mod tests {
rate_limiter: None,
};

let dummy_filename_2 = String::from("test_root_block_device_add_last_2");
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
let dummy_file_2 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_2 = dummy_file_2.path().to_path_buf();
let dummy_block_device_2 = BlockDeviceConfig {
path_on_host: dummy_path_2,
is_root_device: false,
Expand All @@ -347,8 +332,8 @@ mod tests {
rate_limiter: None,
};

let dummy_filename_3 = String::from("test_root_block_device_add_last_3");
let dummy_path_3 = create_dummy_path(dummy_filename_3.clone());
let dummy_file_3 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_3 = dummy_file_3.path().to_path_buf();
let dummy_block_device_3 = BlockDeviceConfig {
path_on_host: dummy_path_3,
is_root_device: false,
Expand All @@ -358,17 +343,17 @@ mod tests {
};

let mut block_devices_configs = BlockDeviceConfigs::new();
let ret2 = block_devices_configs.add(dummy_block_device_2.clone());
let ret3 = block_devices_configs.add(dummy_block_device_3.clone());
let ret1 = block_devices_configs.add(root_block_device.clone());

delete_dummy_path(dummy_filename_1);
delete_dummy_path(dummy_filename_2);
delete_dummy_path(dummy_filename_3);

assert!(ret2.is_ok());
assert!(ret3.is_ok());
assert!(ret1.is_ok());
assert!(
block_devices_configs
.add(dummy_block_device_2.clone())
.is_ok()
);
assert!(
block_devices_configs
.add(dummy_block_device_3.clone())
.is_ok()
);
assert!(block_devices_configs.add(root_block_device.clone()).is_ok());

assert_eq!(block_devices_configs.has_root_block_device(), true);
assert_eq!(block_devices_configs.config_list.len(), 3);
Expand Down Expand Up @@ -400,8 +385,8 @@ mod tests {

#[test]
fn test_update() {
let dummy_filename_1 = String::from("test_update_1");
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
let dummy_file_1 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_1 = dummy_file_1.path().to_path_buf();
let root_block_device = BlockDeviceConfig {
path_on_host: dummy_path_1,
is_root_device: true,
Expand All @@ -410,8 +395,8 @@ mod tests {
rate_limiter: None,
};

let dummy_filename_2 = String::from("test_update_2");
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
let dummy_file_2 = tempfile::NamedTempFile::new().unwrap();
let dummy_path_2 = dummy_file_2.path().to_path_buf();
let mut dummy_block_device_2 = BlockDeviceConfig {
path_on_host: dummy_path_2.clone(),
is_root_device: false,
Expand All @@ -423,31 +408,26 @@ mod tests {
let mut block_devices_configs = BlockDeviceConfigs::new();

// Add 2 block devices
let ret1 = block_devices_configs.add(root_block_device.clone());
let ret2 = block_devices_configs.add(dummy_block_device_2.clone());
assert!(block_devices_configs.add(root_block_device.clone()).is_ok());
assert!(
block_devices_configs
.add(dummy_block_device_2.clone())
.is_ok()
);

// Update OK
dummy_block_device_2.is_read_only = true;
let ret_update_ok = block_devices_configs.update(&dummy_block_device_2);
assert!(block_devices_configs.update(&dummy_block_device_2).is_ok());

// Update with invalid path
let dummy_filename_3 = String::from("test_update_3");
let dummy_path_3 = PathBuf::from(dummy_filename_3.clone());
dummy_block_device_2.path_on_host = dummy_path_3;
let ret_inv_path = block_devices_configs.update(&dummy_block_device_2);
assert!(block_devices_configs.update(&dummy_block_device_2).is_err());

// Update with 2 root block devices
dummy_block_device_2.path_on_host = dummy_path_2;
dummy_block_device_2.is_root_device = true;
let ret_2roots = block_devices_configs.update(&dummy_block_device_2);

delete_dummy_path(dummy_filename_1);
delete_dummy_path(dummy_filename_2);

assert!(ret1.is_ok());
assert!(ret2.is_ok());
assert!(ret_update_ok.is_ok());
assert!(ret_inv_path.is_err());
assert!(ret_2roots.is_err());
assert!(block_devices_configs.update(&dummy_block_device_2).is_err());
}
}
Loading