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

[Martin Kletzander]

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 at gmail.com>
>> > ---
>> > 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211129/026bb227/attachment-0001.sig>