Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Wed, Nov 10, 2021 at 10:24:31PM +0800, Luke Yue wrote:
> Signed-off-by: Luke Yue <lukedyue@xxxxxxxxx>
> ---
> tests/virshtest.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 96 insertions(+)

This makes the tests very fragile.  It would be even easier to do the
test using the internal APIs, similarly to how we do it with the
qemuhotplugtest.

> diff --git a/tests/virshtest.c b/tests/virshtest.c
> index af2a70f5fb..8e5b397420 100644
> --- a/tests/virshtest.c
> +++ b/tests/virshtest.c
> @@ -160,6 +160,8 @@ static char *custom_uri;
>     "--connect", \
>     custom_uri
>
> +# define TEST_XML_PATH abs_top_builddir "/../examples/xml/test"
> +
> static int testCompareListDefault(const void *data G_GNUC_UNUSED)
> {
>     const char *const argv[] = { VIRSH_DEFAULT, "list", NULL };
> @@ -437,6 +439,88 @@ static int testIOThreadPin(const void *data G_GNUC_UNUSED)
>     return testCompareOutputLit(exp, "", NULL, argv);
> }
>
> +static int testCompareDetachDevice(const void *data G_GNUC_UNUSED)
> +{
> +    const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevif.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevdiskcdrom.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevsound.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevhostdev.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevlease.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevcontroller.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevfs.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevrng.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevmem.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevshmem.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevwatchdog.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevinput.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevvsock.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevtpm.xml",
> +                                 NULL };
> +    const char *exp =
> +"Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n";

If any of these fails it will be a pain to figure out which one it was
if the error message does not include the name.  Splitting these is
definitely the way to go.

> +    return testCompareOutputLit(exp, "", NULL, argv);
> +}
> +
> +static int testCompareDetachDeviceError(const void *data G_GNUC_UNUSED)
> +{
> +    const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevtpm.xml;\
> +                                 detach-device fc5\
> +                                 " TEST_XML_PATH "/testdevtpm.xml;\
> +                                 detach-device fc5 --live\
> +                                 " TEST_XML_PATH "/testdevmemballoon.xml",
> +                                 NULL };
> +    const char *exp =
> +"Device detached successfully\n\n\n\n";
> +    const char *error_msg =
> +"error: Failed to detach device from " TEST_XML_PATH "/testdevtpm.xml\n\
> +error: device not found: matching tpm device not found\n\
> +error: Failed to detach device from " TEST_XML_PATH "/testdevmemballoon.xml\n\
> +error: Operation not supported: detach of device 'memballoon' on running domain is not supported\n";

It'd be also nicer to read and write these tests if they did not rely on
the output just like this and instead used the internal APIs.

Attachment: signature.asc
Description: PGP signature