Repository navigation
Conversation
14b4fd7 to
5fb8cee
Compare
sboeuf
left a comment
There was a problem hiding this comment.
Looks okay, but TBH I'm not sure this PR buys us anything. It's barely more readable than the previous versions, and you're loosing the context where it failed through various error messages that you've removed.
| #[cfg(test)] | ||
| pub(crate) fn current_dir(&mut self, directory: impl AsRef<Path>) -> &mut Self { | ||
| self.command.current_dir(directory); | ||
| self | ||
| } |
There was a problem hiding this comment.
Let's not have a dedicated function just for tests.
There was a problem hiding this comment.
I rewrote this by using the explicit image paths, so this function has gone.
Thanks
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
I don't think that's really valuable to have unit tests for this small encapsulation of qemu-img which is useful only through test. I mean testing tests tests 😅
There was a problem hiding this comment.
Gotcha, dropped those :)
Add a shared QemuImg builder under the integration test directory. Include it from block unit tests without adding a crate or dependency. Require the executable and a successful exit for every subcommand. Accept caller context and report the command and stderr on failures. Use the helper for VHDX fixtures and remove skipped fixture tests. Document the executable requirement for local test runs. Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Route all integration image commands through QemuImg. Require a successful exit for setup, info and map commands. Preserve caller operation context alongside command arguments and stderr on failure. Replace shell command strings with argument lists so file paths are passed intact. Share failure reports across disk test helpers. Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Use QemuImg to create the Windows QCOW2 overlay. Keep image creation mandatory and preserve its operation context with command details and stderr on failure. Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Use QemuImg for QCOW2 chain and VHDX fixture generation. Pass full source and output paths while keeping relative QCOW2 backing names in image metadata. Add operation context to fixture failure reports. Require the executable and successful command exit statuses. Fail on fixture file errors instead of returning from tests without running their checks. Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Use QemuImg for disk image setup and compressed image fixtures. Preserve image formats, backing paths, preallocation modes and sizes. Require command success and include operation context, command details and stderr on failure before a benchmark starts. Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
5fb8cee to
ca4aec5
Compare
Thanks for the review. The purpose is to unify the different I have simplified the wrapper and restored caller specific error context through Thanks |
Combine
qemu-imgusages in a sharedQemuImghelper across the test suites. Everyqemu-imgcommand must succeed. A missing executable or nonzero exit status fails the test. Failures report the command and error details.This is prerequisite work for #8707.