[libvirt] [PATCH 4/6] tests: qemumonitorjsontest: Do some actual testing in qemuMonitorJSONTestAttachChardev

[John Ferlan]

On 09/27/2016 12:39 PM, Peter Krempa wrote:
> Until now the test was rather useless since it didn't check the
> arguments formatted and didn't use properly configured chardev objects.
> 
> Add the expected arguments and instrument the test to validate them.
> Modify some test cases to actually add valid data.
> 
> Note that the UDP test data is currently wrong due to a bug.
> ---
>  tests/qemumonitorjsontest.c | 93 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 68 insertions(+), 25 deletions(-)
> 
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 23877f8..61344b7 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -747,6 +747,7 @@ static int
>  qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
>                                      const char *label,
>                                      virDomainChrSourceDefPtr chr,
> +                                    const char *expectargs,
>                                      const char *reply,
>                                      const char *expectPty,
>                                      bool fail)
> @@ -772,7 +773,8 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
>      if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
>          goto cleanup;
> 
> -    if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) < 0)
> +    if (qemuMonitorTestAddItemExpect(data.test, "chardev-add",
> +                                     expectargs, true, jsonreply) < 0)

FWIW: The "knowledge" of whether the expectargs has an apostrophe would
seem to lie in the caller. If in fact you still keep the apostrophe
check in patch 2, then I would think the caller would be able to supply
true/false, but that's a minor thing.

Also, something that just occurred to me... 'expectargs' had better not
be NULL; otherwise, patch 2 will core at the while (*tmp != '\0')

ACK in principle - your call on the apostrophe thing - although perhaps
a check in patch 2 should be added for (apostrophe && data->expectArgs)
to protect from future mishaps...

John

>          goto cleanup;
> 
>      if (virTestRun(fulllabel, &testQemuMonitorJSONAttachChardev, &data) < 0)
> @@ -793,49 +795,90 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt)
>      virDomainChrSourceDef chr;
>      int ret = 0;
> 
> -#define CHECK(label, fail) \
> -    if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, NULL, NULL, \
> -                                            fail) < 0) \
> +#define CHECK(label, fail, expectargs)                                         \
> +    if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, expectargs,   \
> +                                            NULL, NULL, fail) < 0)             \
>          ret = -1
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_NULL };
> -    CHECK("null", false);
> +    CHECK("null", false,
> +          "{'id':'alias','backend':{'type':'null','data':{}}}");
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC };
> -    CHECK("vc", false);
> +    CHECK("vc", false,
> +          "{'id':'alias','backend':{'type':'null','data':{}}}");
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
>      if (qemuMonitorJSONTestAttachOneChardev(xmlopt, "pty", &chr,
> +                                            "{'id':'alias',"
> +                                             "'backend':{'type':'pty',"
> +                                                        "'data':{}}}",
>                                              "\"pty\" : \"/dev/pts/0\"",
>                                              "/dev/pts/0", false) < 0)
>          ret = -1;
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
> -    CHECK("pty missing path", true);
> -
> -    chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_FILE };
> -    CHECK("file", false);
> -
> -    chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_DEV };
> -    CHECK("device", false);
> -
> -    chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_TCP };
> -    CHECK("tcp", false);
> -
> -    chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UDP };
> -    CHECK("udp", false);
> -
> -    chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UNIX };
> -    CHECK("unix", false);
> +    CHECK("pty missing path", true,
> +          "{'id':'alias','backend':{'type':'pty','data':{}}}");
> +
> +    memset(&chr, 0, sizeof(chr));
> +    chr.type = VIR_DOMAIN_CHR_TYPE_FILE;
> +    chr.data.file.path = (char *) "/test/path";
> +    CHECK("file", false,
> +          "{'id':'alias','backend':{'type':'file','data':{'out':'/test/path'}}}");
> +
> +    memset(&chr, 0, sizeof(chr));
> +    chr.type = VIR_DOMAIN_CHR_TYPE_DEV;
> +    chr.data.file.path = (char *) "/test/path";
> +    CHECK("device", false,
> +          "{'id':'alias','backend':{'type':'serial','data':{'device':'/test/path'}}}");
> +
> +    memset(&chr, 0, sizeof(chr));
> +    chr.type = VIR_DOMAIN_CHR_TYPE_TCP;
> +    chr.data.tcp.host = (char *) "example.com";
> +    chr.data.tcp.service = (char *) "1234";
> +    CHECK("tcp", false,
> +          "{'id':'alias',"
> +           "'backend':{'type':'socket',"
> +                      "'data':{'addr':{'type':'inet',"
> +                                      "'data':{'host':'example.com',"
> +                                              "'port':'1234'}},"
> +                              "'wait':false,"
> +                              "'telnet':false,"
> +                              "'server':false}}}");
> +
> +    memset(&chr, 0, sizeof(chr));
> +    chr.type = VIR_DOMAIN_CHR_TYPE_UDP;
> +    chr.data.udp.connectHost = (char *) "example.com";
> +    chr.data.udp.connectService = (char *) "1234";
> +    CHECK("udp", false,
> +          "{'id':'alias',"
> +           "'backend':{'type':'socket',"
> +                      "'data':{'addr':{'type':'inet',"
> +                                      "'data':{'host':'example.com',"
> +                                              "'port':'1234'}}}}}");
> +
> +    memset(&chr, 0, sizeof(chr));
> +    chr.type = VIR_DOMAIN_CHR_TYPE_UNIX;
> +    chr.data.nix.path = (char *) "/path/to/socket";
> +    CHECK("unix", false,
> +          "{'id':'alias',"
> +           "'backend':{'type':'socket',"
> +                      "'data':{'addr':{'type':'unix',"
> +                                      "'data':{'path':'/path/to/socket'}},"
> +                              "'wait':false,"
> +                              "'server':false}}}");
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_SPICEVMC };
> -    CHECK("spicevmc", false);
> +    CHECK("spicevmc", false,
> +          "{'id':'alias','backend':{'type':'spicevmc','"
> +                                    "data':{'type':'vdagent'}}}");
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PIPE };
> -    CHECK("pipe", true);
> +    CHECK("pipe", true, "");
> 
>      chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_STDIO };
> -    CHECK("stdio", true);
> +    CHECK("stdio", true, "");
>  #undef CHECK
> 
>      return ret;
>