Skip to content

Commit c03297d

Browse files
device_config/drive.rs: use tempfile in unit tests
Temfiles are automatically deleted when the object used to create the file gets out of scope. Removed the helpers for creating and deleting files and use named temporary files. Signed-off-by: Andreea Florescu <[email protected]>
1 parent d5c1c41 commit c03297d

File tree

1 file changed

+72
-94
lines changed

1 file changed

+72
-94
lines changed

vmm/src/device_config/drive.rs

Lines changed: 72 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -157,19 +157,7 @@ impl BlockDeviceConfigs {
157157
mod tests {
158158
use super::*;
159159
use api_server::request::sync::{DeviceState, DrivePermissions};
160-
use std::fs::{self, File};
161-
162-
// Helper function for creating a dummy file
163-
// The filename has to be unique among all tests because the tests are run on many threads
164-
fn create_dummy_path(filename: String) -> PathBuf {
165-
let _file = File::create(filename.clone());
166-
return PathBuf::from(filename);
167-
}
168-
169-
// Helper function for deleting a dummy file
170-
fn delete_dummy_path(filename: String) {
171-
let _rs = fs::remove_file(filename);
172-
}
160+
use tempfile::NamedTempFile;
173161

174162
#[test]
175163
fn test_create_block_devices_configs() {
@@ -180,10 +168,9 @@ mod tests {
180168

181169
#[test]
182170
fn test_add_non_root_block_device() {
183-
let dummy_filename = String::from("test_add_non_root_block_device");
184-
let dummy_path = create_dummy_path(dummy_filename.clone());
171+
let dummy_file = NamedTempFile::new().unwrap();
172+
let dummy_path = dummy_file.path().to_path_buf();
185173
let dummy_id = String::from("1");
186-
187174
let dummy_block_device = BlockDeviceConfig {
188175
path_on_host: dummy_path.clone(),
189176
is_root_device: false,
@@ -193,25 +180,24 @@ mod tests {
193180
};
194181

195182
let mut block_devices_configs = BlockDeviceConfigs::new();
196-
let ret = block_devices_configs.add(dummy_block_device.clone());
197-
// clean up before the assert!s to make sure the temp files are deleted
198-
delete_dummy_path(dummy_filename);
199-
assert!(ret.is_ok());
183+
assert_eq!(
184+
block_devices_configs.add(dummy_block_device.clone()),
185+
Ok(())
186+
);
200187

201188
assert_eq!(block_devices_configs.has_root_block, false);
202189
assert_eq!(block_devices_configs.config_list.len(), 1);
190+
203191
let dev_config = block_devices_configs.config_list.iter().next().unwrap();
204192
assert_eq!(dev_config, &dummy_block_device);
205193
assert!(block_devices_configs.contains_drive_path(dummy_path));
206-
assert!(!block_devices_configs.contains_drive_path(PathBuf::from("/foo/bar")));
207-
assert!(block_devices_configs.contains_drive_id(dummy_id.clone()));
208-
assert!(!block_devices_configs.contains_drive_id(String::from("foo")));
194+
assert!(block_devices_configs.contains_drive_id(dummy_id));
209195
}
210196

211197
#[test]
212198
fn test_add_one_root_block_device() {
213-
let dummy_filename = String::from("test_add_one_root_block_device");
214-
let dummy_path = create_dummy_path(dummy_filename.clone());
199+
let dummy_file = NamedTempFile::new().unwrap();
200+
let dummy_path = dummy_file.path().to_path_buf();
215201

216202
let dummy_block_device = BlockDeviceConfig {
217203
path_on_host: dummy_path,
@@ -220,10 +206,13 @@ mod tests {
220206
drive_id: String::from("1"),
221207
rate_limiter: None,
222208
};
209+
223210
let mut block_devices_configs = BlockDeviceConfigs::new();
224-
let ret = block_devices_configs.add(dummy_block_device.clone());
225-
delete_dummy_path(dummy_filename);
226-
assert!(ret.is_ok());
211+
assert!(
212+
block_devices_configs
213+
.add(dummy_block_device.clone())
214+
.is_ok()
215+
);
227216

228217
assert_eq!(block_devices_configs.has_root_block, true);
229218
assert_eq!(block_devices_configs.config_list.len(), 1);
@@ -234,8 +223,8 @@ mod tests {
234223

235224
#[test]
236225
fn test_add_two_root_block_devices_configs() {
237-
let dummy_filename_1 = String::from("test_add_two_root_block_devices_configs_1");
238-
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
226+
let dummy_file_1 = NamedTempFile::new().unwrap();
227+
let dummy_path_1 = dummy_file_1.path().to_path_buf();
239228
let root_block_device_1 = BlockDeviceConfig {
240229
path_on_host: dummy_path_1,
241230
is_root_device: true,
@@ -244,8 +233,8 @@ mod tests {
244233
rate_limiter: None,
245234
};
246235

247-
let dummy_filename_2 = String::from("test_add_two_root_block_devices_configs_2");
248-
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
236+
let dummy_file_2 = NamedTempFile::new().unwrap();
237+
let dummy_path_2 = dummy_file_2.path().to_path_buf();
249238
let root_block_device_2 = BlockDeviceConfig {
250239
path_on_host: dummy_path_2,
251240
is_root_device: true,
@@ -255,25 +244,18 @@ mod tests {
255244
};
256245

257246
let mut block_devices_configs = BlockDeviceConfigs::new();
258-
let ret = block_devices_configs.add(root_block_device_1);
259-
let actual_error = format!(
260-
"{:?}",
261-
block_devices_configs.add(root_block_device_2).unwrap_err()
247+
assert!(block_devices_configs.add(root_block_device_1).is_ok());
248+
assert_eq!(
249+
block_devices_configs.add(root_block_device_2).unwrap_err(),
250+
DriveError::RootBlockDeviceAlreadyAdded
262251
);
263-
let expected_error = format!("{:?}", DriveError::RootBlockDeviceAlreadyAdded);
264-
265-
delete_dummy_path(dummy_filename_1);
266-
delete_dummy_path(dummy_filename_2);
267-
268-
assert!(ret.is_ok());
269-
assert_eq!(expected_error, actual_error);
270252
}
271253

272254
#[test]
273255
/// Test BlockDevicesConfigs::add when you first add the root device and then the other devices
274256
fn test_add_root_block_device_first() {
275-
let dummy_filename_1 = String::from("test_add_root_block_device_first_1");
276-
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
257+
let dummy_file_1 = NamedTempFile::new().unwrap();
258+
let dummy_path_1 = dummy_file_1.path().to_path_buf();
277259
let root_block_device = BlockDeviceConfig {
278260
path_on_host: dummy_path_1,
279261
is_root_device: true,
@@ -282,8 +264,8 @@ mod tests {
282264
rate_limiter: None,
283265
};
284266

285-
let dummy_filename_2 = String::from("test_add_root_block_device_first_2");
286-
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
267+
let dummy_file_2 = NamedTempFile::new().unwrap();
268+
let dummy_path_2 = dummy_file_2.path().to_path_buf();
287269
let dummy_block_device_2 = BlockDeviceConfig {
288270
path_on_host: dummy_path_2,
289271
is_root_device: false,
@@ -292,8 +274,8 @@ mod tests {
292274
rate_limiter: None,
293275
};
294276

295-
let dummy_filename_3 = String::from("test_add_root_block_device_first_3");
296-
let dummy_path_3 = create_dummy_path(dummy_filename_3.clone());
277+
let dummy_file_3 = NamedTempFile::new().unwrap();
278+
let dummy_path_3 = dummy_file_3.path().to_path_buf();
297279
let dummy_block_device_3 = BlockDeviceConfig {
298280
path_on_host: dummy_path_3,
299281
is_root_device: false,
@@ -303,17 +285,18 @@ mod tests {
303285
};
304286

305287
let mut block_devices_configs = BlockDeviceConfigs::new();
306-
let ret1 = block_devices_configs.add(root_block_device.clone());
307-
let ret2 = block_devices_configs.add(dummy_block_device_2.clone());
308-
let ret3 = block_devices_configs.add(dummy_block_device_3.clone());
309288

310-
delete_dummy_path(dummy_filename_1);
311-
delete_dummy_path(dummy_filename_2);
312-
delete_dummy_path(dummy_filename_3);
313-
314-
assert!(ret1.is_ok());
315-
assert!(ret2.is_ok());
316-
assert!(ret3.is_ok());
289+
assert!(block_devices_configs.add(root_block_device.clone()).is_ok());
290+
assert!(
291+
block_devices_configs
292+
.add(dummy_block_device_2.clone())
293+
.is_ok()
294+
);
295+
assert!(
296+
block_devices_configs
297+
.add(dummy_block_device_3.clone())
298+
.is_ok()
299+
);
317300

318301
assert_eq!(block_devices_configs.has_root_block_device(), true);
319302
assert_eq!(block_devices_configs.config_list.len(), 3);
@@ -327,8 +310,8 @@ mod tests {
327310
#[test]
328311
/// Test BlockDevicesConfigs::add when you add other devices first and then the root device
329312
fn test_root_block_device_add_last() {
330-
let dummy_filename_1 = String::from("test_root_block_device_add_last_1");
331-
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
313+
let dummy_file_1 = NamedTempFile::new().unwrap();
314+
let dummy_path_1 = dummy_file_1.path().to_path_buf();
332315
let root_block_device = BlockDeviceConfig {
333316
path_on_host: dummy_path_1,
334317
is_root_device: true,
@@ -337,8 +320,8 @@ mod tests {
337320
rate_limiter: None,
338321
};
339322

340-
let dummy_filename_2 = String::from("test_root_block_device_add_last_2");
341-
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
323+
let dummy_file_2 = NamedTempFile::new().unwrap();
324+
let dummy_path_2 = dummy_file_2.path().to_path_buf();
342325
let dummy_block_device_2 = BlockDeviceConfig {
343326
path_on_host: dummy_path_2,
344327
is_root_device: false,
@@ -347,8 +330,8 @@ mod tests {
347330
rate_limiter: None,
348331
};
349332

350-
let dummy_filename_3 = String::from("test_root_block_device_add_last_3");
351-
let dummy_path_3 = create_dummy_path(dummy_filename_3.clone());
333+
let dummy_file_3 = NamedTempFile::new().unwrap();
334+
let dummy_path_3 = dummy_file_3.path().to_path_buf();
352335
let dummy_block_device_3 = BlockDeviceConfig {
353336
path_on_host: dummy_path_3,
354337
is_root_device: false,
@@ -358,17 +341,17 @@ mod tests {
358341
};
359342

360343
let mut block_devices_configs = BlockDeviceConfigs::new();
361-
let ret2 = block_devices_configs.add(dummy_block_device_2.clone());
362-
let ret3 = block_devices_configs.add(dummy_block_device_3.clone());
363-
let ret1 = block_devices_configs.add(root_block_device.clone());
364-
365-
delete_dummy_path(dummy_filename_1);
366-
delete_dummy_path(dummy_filename_2);
367-
delete_dummy_path(dummy_filename_3);
368-
369-
assert!(ret2.is_ok());
370-
assert!(ret3.is_ok());
371-
assert!(ret1.is_ok());
344+
assert!(
345+
block_devices_configs
346+
.add(dummy_block_device_2.clone())
347+
.is_ok()
348+
);
349+
assert!(
350+
block_devices_configs
351+
.add(dummy_block_device_3.clone())
352+
.is_ok()
353+
);
354+
assert!(block_devices_configs.add(root_block_device.clone()).is_ok());
372355

373356
assert_eq!(block_devices_configs.has_root_block_device(), true);
374357
assert_eq!(block_devices_configs.config_list.len(), 3);
@@ -400,8 +383,8 @@ mod tests {
400383

401384
#[test]
402385
fn test_update() {
403-
let dummy_filename_1 = String::from("test_update_1");
404-
let dummy_path_1 = create_dummy_path(dummy_filename_1.clone());
386+
let dummy_file_1 = NamedTempFile::new().unwrap();
387+
let dummy_path_1 = dummy_file_1.path().to_path_buf();
405388
let root_block_device = BlockDeviceConfig {
406389
path_on_host: dummy_path_1,
407390
is_root_device: true,
@@ -410,8 +393,8 @@ mod tests {
410393
rate_limiter: None,
411394
};
412395

413-
let dummy_filename_2 = String::from("test_update_2");
414-
let dummy_path_2 = create_dummy_path(dummy_filename_2.clone());
396+
let dummy_file_2 = NamedTempFile::new().unwrap();
397+
let dummy_path_2 = dummy_file_2.path().to_path_buf();
415398
let mut dummy_block_device_2 = BlockDeviceConfig {
416399
path_on_host: dummy_path_2.clone(),
417400
is_root_device: false,
@@ -423,31 +406,26 @@ mod tests {
423406
let mut block_devices_configs = BlockDeviceConfigs::new();
424407

425408
// Add 2 block devices
426-
let ret1 = block_devices_configs.add(root_block_device.clone());
427-
let ret2 = block_devices_configs.add(dummy_block_device_2.clone());
409+
assert!(block_devices_configs.add(root_block_device.clone()).is_ok());
410+
assert!(
411+
block_devices_configs
412+
.add(dummy_block_device_2.clone())
413+
.is_ok()
414+
);
428415

429416
// Update OK
430417
dummy_block_device_2.is_read_only = true;
431-
let ret_update_ok = block_devices_configs.update(&dummy_block_device_2);
418+
assert!(block_devices_configs.update(&dummy_block_device_2).is_ok());
432419

433420
// Update with invalid path
434421
let dummy_filename_3 = String::from("test_update_3");
435422
let dummy_path_3 = PathBuf::from(dummy_filename_3.clone());
436423
dummy_block_device_2.path_on_host = dummy_path_3;
437-
let ret_inv_path = block_devices_configs.update(&dummy_block_device_2);
424+
assert!(block_devices_configs.update(&dummy_block_device_2).is_err());
438425

439426
// Update with 2 root block devices
440427
dummy_block_device_2.path_on_host = dummy_path_2;
441428
dummy_block_device_2.is_root_device = true;
442-
let ret_2roots = block_devices_configs.update(&dummy_block_device_2);
443-
444-
delete_dummy_path(dummy_filename_1);
445-
delete_dummy_path(dummy_filename_2);
446-
447-
assert!(ret1.is_ok());
448-
assert!(ret2.is_ok());
449-
assert!(ret_update_ok.is_ok());
450-
assert!(ret_inv_path.is_err());
451-
assert!(ret_2roots.is_err());
429+
assert!(block_devices_configs.update(&dummy_block_device_2).is_err());
452430
}
453431
}

0 commit comments

Comments
 (0)