9 a.m.
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(a)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.