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;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list