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

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