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

Luke Yue posted 12 patches 4 years, 2 months ago
There is a newer version of this series
[PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
Posted by Luke Yue 4 years, 2 months ago
Signed-off-by: Luke Yue <lukedyue@gmail.com>
---
 tests/virshtest.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

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";
+    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";
+    return testCompareOutputLit(exp, error_msg, NULL, argv);
+}
+
+static int testCompareDetachDeviceAlias(const void *data G_GNUC_UNUSED)
+{
+    const char *const argv[] = { VIRSH_CUSTOM,
+                                 "detach-device-alias fc5 ua-testCD;\
+                                 detach-device fc5\
+                                 " TEST_XML_PATH "/testdevdiskcdrom.xml",
+                                 NULL };
+    const char *exp = "Device detach request sent successfully\n\n\n";
+    const char *error_msg =
+"error: Failed to detach device from " TEST_XML_PATH "/testdevdiskcdrom.xml\n\
+error: device not found: no target device hdb\n";
+    return testCompareOutputLit(exp, error_msg, NULL, argv);
+}
+
 struct testInfo {
     const char *const *argv;
     const char *result;
@@ -553,6 +637,18 @@ mymain(void)
                    testIOThreadPin, NULL) != 0)
         ret = -1;
 
+    if (virTestRun("virsh detach-device",
+                   testCompareDetachDevice, NULL) != 0)
+        ret = -1;
+
+    if (virTestRun("virsh detach-device (with failure)",
+                   testCompareDetachDeviceError, NULL) != 0)
+        ret = -1;
+
+    if (virTestRun("virsh detach-device-alias",
+                   testCompareDetachDeviceAlias, NULL) != 0)
+        ret = -1;
+
     /* It's a bit awkward listing result before argument, but that's a
      * limitation of C99 vararg macros.  */
 # define DO_TEST(i, result, ...) \
-- 
2.33.1

Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
Posted by Martin Kletzander 4 years, 2 months ago
On Wed, Nov 10, 2021 at 10:24:31PM +0800, Luke Yue wrote:
>Signed-off-by: Luke Yue <lukedyue@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.

>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.
Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
Posted by Luke Yue 4 years, 2 months ago
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@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?

Thanks,
Luke

Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
Posted by Martin Kletzander 4 years, 2 months ago
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@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
>
Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
Posted by Luke Yue 4 years, 2 months ago
On Mon, 2021-11-29 at 15:50 +0100, Martin Kletzander wrote:
> 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@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, will reimplement these tests with internal APIs in next
version.