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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 29, 2021 at 10:41:51PM +0800, Luke Yue wrote:
On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote:
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.


Thanks for the explanation, I will look into how qemuhotplugtest is
implemented.

> 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.

Should I create a new file for these tests? As this file is for `virsh`
test but no for test using internal APIs?


I would go with "generichotplugtest" or something like that.

Thanks,
Luke

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux