Extract qemuimg test package #37
Conversation
go.mod
Outdated
| // Testing requirements | ||
| require github.com/lima-vm/go-qcow2reader/test v0.4.0 | ||
|
|
||
| replace github.com/lima-vm/go-qcow2reader/test v0.4.0 => ./test |
There was a problem hiding this comment.
Why this has to be a separate module, not just a separate package?
There was a problem hiding this comment.
It can be in the package, I tried to avoid building this as part of the go-qcow2reader module. Removing the extra module will make it simpler.
There was a problem hiding this comment.
I don't like maintaining multiple modules (with an exception for cmd/go-qcow2reader-example, as it was inevitable for isolating the third-party zstd dependency from the core module).
IIUC, just splitting the package is enough to omit the corresponding object code from the binary of Lima.
Even if it was not enough, the footprint of the object code would be negligible (just a few KiB?)
There was a problem hiding this comment.
The qemuimg package is tiny. I'll remove the module. Should we keep the test directory to make it more clear that this is a test only code?
There was a problem hiding this comment.
Should we keep the test directory to make it more clear that this is a test only code?
Either would be fine
| @@ -1,26 +1,21 @@ | |||
| package qcow2reader | |||
| package qcow2reader_test | |||
There was a problem hiding this comment.
The test should use the same package as the code, no?
There was a problem hiding this comment.
This is for whitebox tests that need access to the internals. For blackbox tests, it is better to have the test in the special *_test package. This way we use the code in the same way a user would use the code.
I think the default should be blackbox test, and whitebox tests should be used only if needed.
There was a problem hiding this comment.
Thanks, could you add that context as a code comment?
nirs
changed the title
qcow2reader_test.go
Outdated
| @@ -1,26 +1,22 @@ | |||
| package qcow2reader | |||
| // package qcow2reader_test keeps blackbox tests for qcow2reader. | |||
There was a problem hiding this comment.
nit
| // package qcow2reader_test keeps blackbox tests for qcow2reader. | |
| // Package qcow2reader_test keeps blackbox tests for qcow2reader. |
The qemuimg package provides now only qemuimg.Convert(). I plan to add qemuimg.Create(), qemuimg.Info(), qemuimg.Map(), and qemuimg.Compare(). This makes the code nicer to work with, but adds a test only dependency. The qcow2reader tests use now qemu2reader_test package, so the dependency should be built only for tests. The qemuimg test package will also be useful for other project using this library, since testing code using the library typically requires creating, converting and comparing qcow2 images. Signed-off-by: Nir Soffer <[email protected]>
2 participants
The qemuimg package provides now only qemuimg.Convert(). I plan to add
qemuimg.Create(), qemuimg.Info(), qemuimg.Map(), and qemuimg.Compare().
This makes the code nicer to work with, but adds a test only dependency.
The qcow2reader tests use now qemu2reader_test package, so the
dependency should be built only for tests.
The qemuimg test package will also be useful for other project using
this library, since testing code using the library typically requires
creating, converting and comparing qcow2 images.