[libvirt] [PATCH 0/6] tests: qemu: Fix testing QMP commands against the schema

I was trying to add some new commands and made a typo. The tests did not catch it. I must have messed up something when sending the original schema validation impl. Peter Krempa (6): tests: qemuschema: Fix copy-paste error in function name tests: qemuschema: Add line break to debug message tests: qemumonitorutils: Don't crash on wrong monitor command tests: qemumonitorjson: Raise the necessary debug level for QAPI schema checks tests: qemumonitorjson: Fix schema testing of monitor commands tests: qemumonitorjson: Do QMP schema validation for DO_TEST_GEN tests/qemumonitorjsontest.c | 10 ++++++---- tests/qemumonitortestutils.c | 3 ++- tests/testutilsqemuschema.c | 10 +++++----- 3 files changed, 13 insertions(+), 10 deletions(-) -- 2.16.2

s/testQEMUSchemaValidateArrayBuiltin/testQEMUSchemaValidateBuiltin/ Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index 1cec5265e1..fb22803a22 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -28,9 +28,9 @@ testQEMUSchemaValidateRecurse(virJSONValuePtr obj, virBufferPtr debug); static int -testQEMUSchemaValidateArrayBuiltin(virJSONValuePtr obj, - virJSONValuePtr root, - virBufferPtr debug) +testQEMUSchemaValidateBuiltin(virJSONValuePtr obj, + virJSONValuePtr root, + virBufferPtr debug) { const char *t = virJSONValueObjectGetString(root, "json-type"); const char *s = NULL; @@ -476,7 +476,7 @@ testQEMUSchemaValidateRecurse(virJSONValuePtr obj, const char *t = virJSONValueObjectGetString(root, "meta-type"); if (STREQ_NULLABLE(t, "builtin")) { - return testQEMUSchemaValidateArrayBuiltin(obj, root, debug); + return testQEMUSchemaValidateBuiltin(obj, root, debug); } else if (STREQ_NULLABLE(t, "object")) { return testQEMUSchemaValidateObject(obj, root, schema, debug); } else if (STREQ_NULLABLE(t, "enum")) { -- 2.16.2

Message stating which schema replies file is being used would be squashed with other messages. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index fb22803a22..aa846e1e79 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -539,7 +539,7 @@ testQEMUSchemaGetLatest(void) return NULL; } - VIR_TEST_DEBUG("replies file: '%s'", capsLatestFile); + VIR_TEST_DEBUG("replies file: '%s'\n", capsLatestFile); if (virTestLoadFile(capsLatestFile, &capsLatest) < 0) goto cleanup; -- 2.16.2

virQEMUQAPISchemaPathGet returns success when a given schema path was not found but the returned object is set to NULL. This meant that we'd call testQEMUSchemaValidate with the schemaroot being NULL which lead to a crash if a mistyped monitor command was tested. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitortestutils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index d857c381a4..15eba5898e 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -564,7 +564,8 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, if (virAsprintf(&schemapath, "%s/arg-type", cmdname) < 0) goto cleanup; - if (virQEMUQAPISchemaPathGet(schemapath, test->qapischema, &schemaroot) < 0) { + if (virQEMUQAPISchemaPathGet(schemapath, test->qapischema, &schemaroot) < 0 || + !schemaroot) { if (qemuMonitorReportError(test, "command '%s' not found in QAPI schema", cmdname) == 0) -- 2.16.2

The debug output of the schema validator on success is not so interresting that it should be printed when basic debugging is enabled. Print it only when test debugging is set to 3 and more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 9039cef423..fce2108932 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2838,7 +2838,7 @@ testQAPISchema(const void *opaque) ret = 0; } - if (virTestGetDebug() || + if (virTestGetDebug() >= 3 || (ret < 0 && virTestGetVerbose())) { char *debugstr = virBufferContentAndReset(&debug); fprintf(stderr, "\n%s\n", debugstr); -- 2.16.2

On Thu, Jul 12, 2018 at 01:58:42PM +0200, Peter Krempa wrote:
The debug output of the schema validator on success is not so interresting that it should be printed when basic debugging is enabled.
*interesting
Print it only when test debugging is set to 3 and more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Jano

The 'simpleFunc' data structure is overwritten by the code generated from the macros which initiate the tests. This means that most of the tests would get NULL 'schema' member which means that the schema validation would not take place. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index fce2108932..95b718ab37 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2879,7 +2879,6 @@ mymain(void) ret = -1; goto cleanup; } - simpleFunc.schema = qapiData.schema; #define DO_TEST(name) \ if (virTestRun(# name, testQemuMonitorJSON ## name, driver.xmlopt) < 0) \ @@ -2887,7 +2886,9 @@ mymain(void) #define DO_TEST_SIMPLE(CMD, FNC, ...) \ simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.cmd = CMD, .func = FNC, \ - .xmlopt = driver.xmlopt, __VA_ARGS__ }; \ + .xmlopt = driver.xmlopt, \ + .schema = qapiData.schema, \ + __VA_ARGS__ }; \ if (virTestRun(# FNC, testQemuMonitorJSONSimpleFunc, &simpleFunc) < 0) \ ret = -1 -- 2.16.2

Test some more QMP commands against the schema. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 95b718ab37..e9b2632655 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1297,7 +1297,7 @@ testQemuMonitorJSON ## funcName(const void *opaque) \ { \ const testQemuMonitorJSONSimpleFuncData *data = opaque; \ virDomainXMLOptionPtr xmlopt = data->xmlopt; \ - qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); \ + qemuMonitorTestPtr test = qemuMonitorTestNewSchema(xmlopt, data->schema); \ const char *reply = data->reply; \ int ret = -1; \ \ @@ -2894,6 +2894,7 @@ mymain(void) #define DO_TEST_GEN(name, ...) \ simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.xmlopt = driver.xmlopt, \ + .schema = qapiData.schema \ __VA_ARGS__ }; \ if (virTestRun(# name, testQemuMonitorJSON ## name, &simpleFunc) < 0) \ ret = -1 -- 2.16.2

On Thu, Jul 12, 2018 at 01:58:38PM +0200, Peter Krempa wrote:
I was trying to add some new commands and made a typo. The tests did not catch it. I must have messed up something when sending the original schema validation impl.
Peter Krempa (6): tests: qemuschema: Fix copy-paste error in function name tests: qemuschema: Add line break to debug message tests: qemumonitorutils: Don't crash on wrong monitor command tests: qemumonitorjson: Raise the necessary debug level for QAPI schema checks tests: qemumonitorjson: Fix schema testing of monitor commands tests: qemumonitorjson: Do QMP schema validation for DO_TEST_GEN
tests/qemumonitorjsontest.c | 10 ++++++---- tests/qemumonitortestutils.c | 3 ++- tests/testutilsqemuschema.c | 10 +++++----- 3 files changed, 13 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Peter Krempa