-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Added some more vmm unit tests #367
Conversation
andreeaflorescu
commented
Jun 21, 2018
- Added unit tests tests for put_block_device and put_net_device.
- Removed the helper functions for creating and deleting files in unit tests. Used tempfile crate instead because it handles this automatically.
rx_rate_limiter: None, | ||
tx_rate_limiter: None, | ||
}; | ||
match vmm.put_net_device(network_interface) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this really work? I remember there was a problem with updating network interfaces because of it attempts to take() the tap again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends what you update. If you have no mac address initially, the update will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update issue is addressed by #370 (which is currently failing tests because I created the PR from the wrong branch and will be corrected shortly).
2557775
to
c03297d
Compare
vmm/src/device_config/drive.rs
Outdated
// clean up before the assert!s to make sure the temp files are deleted | ||
delete_dummy_path(dummy_filename); | ||
assert!(ret.is_ok()); | ||
assert_eq!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not assert!(block_devices_configs.add(dummy_block_device.clone()).is_ok())
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a miss on my side. I will change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still assert_eq!
, it should be assert!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed it. take a look.
let ret = block_devices_configs.add(dummy_block_device.clone()); | ||
delete_dummy_path(dummy_filename); | ||
assert!(ret.is_ok()); | ||
assert!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assert
s on is_ok
(and not on _eq(Ok())
), please change the previous one to do the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do.
c03297d
to
fdca141
Compare
fdca141
to
e9063b5
Compare
Added unit tests for put_block_device and put_net_device. Added a new crate tempfile for testing the block device with temporary files that are deleted when the object gets out of scope. Signed-off-by: Andreea Florescu <[email protected]>
8980b1f
to
6b2c2f3
Compare
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]>
6b2c2f3
to
5c4793f
Compare