[libvirt] [PATCH 0/5] tests: json: Improve testing of parsing and formatting

For an experiment I was doing I needed to nest JSON string into an attribute of a different JSON string, so I wrote some tests for that. The experiment failed, but the tests may make sense to have in libvirt. Peter Krempa (5): tests: Rename jsontest to virjsontest tests: virjson: Modify logic in testJSONFromString tests: virjson: Remove spaces from 'very-hard' parsing example tests: virjson: Test formatting along with parsing of JSON objects tests: virjson: Test string escaping tests/Makefile.am | 8 +- tests/{jsontest.c => virjsontest.c} | 195 +++++++++++++++++++++++++++--------- 2 files changed, 150 insertions(+), 53 deletions(-) rename tests/{jsontest.c => virjsontest.c} (69%) -- 2.12.2

--- tests/Makefile.am | 8 ++++---- tests/{jsontest.c => virjsontest.c} | 0 2 files changed, 4 insertions(+), 4 deletions(-) rename tests/{jsontest.c => virjsontest.c} (100%) diff --git a/tests/Makefile.am b/tests/Makefile.am index 19986dc99..3596b5ff1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -328,7 +328,7 @@ test_programs += objectlocking endif WITH_CIL if WITH_YAJL -test_programs += jsontest +test_programs += virjsontest endif WITH_YAJL test_programs += \ @@ -1375,9 +1375,9 @@ virfirewalltest_SOURCES = \ virfirewalltest_LDADD = $(LDADDS) $(DBUS_LIBS) virfirewalltest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) -jsontest_SOURCES = \ - jsontest.c testutils.h testutils.c -jsontest_LDADD = $(LDADDS) +virjsontest_SOURCES = \ + virjsontest.c testutils.h testutils.c +virjsontest_LDADD = $(LDADDS) utiltest_SOURCES = \ utiltest.c testutils.h testutils.c diff --git a/tests/jsontest.c b/tests/virjsontest.c similarity index 100% rename from tests/jsontest.c rename to tests/virjsontest.c -- 2.12.2

To allow better testing in case where the string was parsed, modify the logic so that the regular code path is not included in a conditional block. --- tests/virjsontest.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/virjsontest.c b/tests/virjsontest.c index b67f68ccb..9ecde680f 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -25,24 +25,24 @@ testJSONFromString(const void *data) json = virJSONValueFromString(info->doc); - if (info->pass) { - if (!json) { + if (!json) { + if (info->pass) { VIR_TEST_VERBOSE("Fail to parse %s\n", info->doc); - ret = -1; - goto cleanup; - } else { - VIR_TEST_DEBUG("Parsed %s\n", info->doc); - } - } else { - if (json) { - VIR_TEST_VERBOSE("Should not have parsed %s\n", info->doc); - ret = -1; goto cleanup; } else { VIR_TEST_DEBUG("Fail to parse %s\n", info->doc); + ret = 0; + goto cleanup; } } + if (!info->pass) { + VIR_TEST_VERBOSE("Should not have parsed %s\n", info->doc); + goto cleanup; + } + + VIR_TEST_DEBUG("Parsed %s\n", info->doc); + ret = 0; cleanup: -- 2.12.2

The example is rather long and upcomming patch will check whether the string can be formatted back. As the formatted string lacks spaces and adding the 'expect' string with spaces would be rather long, just drop spaces from this test case. There are other test cases which do contain spaces. --- tests/virjsontest.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/tests/virjsontest.c b/tests/virjsontest.c index 9ecde680f..2a6b2f44e 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -341,30 +341,30 @@ mymain(void) "\"label\": \"charmonitor\"}, {\"filename\": \"pty:/dev/pts/158\"," "\"label\": \"charserial0\"}], \"id\": \"libvirt-3\"}"); - DO_TEST_PARSE("VeryHard", "{\"return\": [{\"name\": \"quit\"}, {\"name\":" - "\"eject\"}, {\"name\": \"change\"}, {\"name\": \"screendump\"}," - "{\"name\": \"stop\"}, {\"name\": \"cont\"}, {\"name\": " - "\"system_reset\"}, {\"name\": \"system_powerdown\"}, " - "{\"name\": \"device_add\"}, {\"name\": \"device_del\"}, " - "{\"name\": \"cpu\"}, {\"name\": \"memsave\"}, {\"name\": " - "\"pmemsave\"}, {\"name\": \"migrate\"}, {\"name\": " - "\"migrate_cancel\"}, {\"name\": \"migrate_set_speed\"}," - "{\"name\": \"client_migrate_info\"}, {\"name\": " - "\"migrate_set_downtime\"}, {\"name\": \"netdev_add\"}, " - "{\"name\": \"netdev_del\"}, {\"name\": \"block_resize\"}," - "{\"name\": \"balloon\"}, {\"name\": \"set_link\"}, {\"name\":" - "\"getfd\"}, {\"name\": \"closefd\"}, {\"name\": \"block_passwd\"}," - "{\"name\": \"set_password\"}, {\"name\": \"expire_password\"}," - "{\"name\": \"qmp_capabilities\"}, {\"name\": " - "\"human-monitor-command\"}, {\"name\": \"query-version\"}," - "{\"name\": \"query-commands\"}, {\"name\": \"query-chardev\"}," - "{\"name\": \"query-block\"}, {\"name\": \"query-blockstats\"}, " - "{\"name\": \"query-cpus\"}, {\"name\": \"query-pci\"}, {\"name\":" - "\"query-kvm\"}, {\"name\": \"query-status\"}, {\"name\": " - "\"query-mice\"}, {\"name\": \"query-vnc\"}, {\"name\": " - "\"query-spice\"}, {\"name\": \"query-name\"}, {\"name\": " - "\"query-uuid\"}, {\"name\": \"query-migrate\"}, {\"name\": " - "\"query-balloon\"}], \"id\": \"libvirt-2\"}"); + DO_TEST_PARSE("VeryHard", "{\"return\":[{\"name\":\"quit\"},{\"name\":" + "\"eject\"},{\"name\":\"change\"},{\"name\":\"screendump\"}," + "{\"name\":\"stop\"},{\"name\":\"cont\"},{\"name\":" + "\"system_reset\"},{\"name\":\"system_powerdown\"}," + "{\"name\":\"device_add\"},{\"name\":\"device_del\"}," + "{\"name\":\"cpu\"},{\"name\":\"memsave\"},{\"name\":" + "\"pmemsave\"},{\"name\":\"migrate\"},{\"name\":" + "\"migrate_cancel\"},{\"name\":\"migrate_set_speed\"}," + "{\"name\":\"client_migrate_info\"},{\"name\":" + "\"migrate_set_downtime\"},{\"name\":\"netdev_add\"}," + "{\"name\":\"netdev_del\"},{\"name\":\"block_resize\"}," + "{\"name\":\"balloon\"},{\"name\":\"set_link\"},{\"name\":" + "\"getfd\"},{\"name\":\"closefd\"},{\"name\":\"block_passwd\"}," + "{\"name\":\"set_password\"},{\"name\":\"expire_password\"}," + "{\"name\":\"qmp_capabilities\"},{\"name\":" + "\"human-monitor-command\"},{\"name\":\"query-version\"}," + "{\"name\":\"query-commands\"},{\"name\":\"query-chardev\"}," + "{\"name\":\"query-block\"},{\"name\":\"query-blockstats\"}," + "{\"name\":\"query-cpus\"},{\"name\":\"query-pci\"},{\"name\":" + "\"query-kvm\"},{\"name\":\"query-status\"},{\"name\":" + "\"query-mice\"},{\"name\":\"query-vnc\"},{\"name\":" + "\"query-spice\"},{\"name\":\"query-name\"},{\"name\":" + "\"query-uuid\"},{\"name\":\"query-migrate\"},{\"name\":" + "\"query-balloon\"}],\"id\":\"libvirt-2\"}"); DO_TEST_FULL("add and remove", AddRemove, "{\"name\": \"sample\", \"value\": true}", -- 2.12.2

Format the parsed string back and compare it to the original (or modified) string for back and forth comparison. --- tests/virjsontest.c | 58 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/tests/virjsontest.c b/tests/virjsontest.c index 2a6b2f44e..451275a4c 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -21,6 +21,8 @@ testJSONFromString(const void *data) { const struct testInfo *info = data; virJSONValuePtr json; + const char *expectstr = info->expect ? info->expect : info->doc; + char *formatted = NULL; int ret = -1; json = virJSONValueFromString(info->doc); @@ -43,9 +45,20 @@ testJSONFromString(const void *data) VIR_TEST_DEBUG("Parsed %s\n", info->doc); + if (!(formatted = virJSONValueToString(json, false))) { + VIR_TEST_VERBOSE("Failed to format json data\n"); + goto cleanup; + } + + if (STRNEQ(expectstr, formatted)) { + virTestDifference(stderr, expectstr, formatted); + goto cleanup; + } + ret = 0; cleanup: + VIR_FREE(formatted); virJSONValueFree(json); return ret; } @@ -323,23 +336,39 @@ mymain(void) ret = -1; \ } while (0) -#define DO_TEST_PARSE(name, doc) \ - DO_TEST_FULL(name, FromString, doc, NULL, true) +/** + * DO_TEST_PARSE: + * @name: test name + * @doc: source JSON string + * @expect: expected output JSON formatted from parsed @doc + * + * Parses @doc and formats it back. If @expect is NULL the result has to be + * identical to @doc. + */ +#define DO_TEST_PARSE(name, doc, expect) \ + DO_TEST_FULL(name, FromString, doc, expect, true) #define DO_TEST_PARSE_FAIL(name, doc) \ DO_TEST_FULL(name, FromString, doc, NULL, false) - DO_TEST_PARSE("Simple", "{\"return\": {}, \"id\": \"libvirt-1\"}"); + DO_TEST_PARSE("Simple", "{\"return\": {}, \"id\": \"libvirt-1\"}", + "{\"return\":{},\"id\":\"libvirt-1\"}"); DO_TEST_PARSE("NotSoSimple", "{\"QMP\": {\"version\": {\"qemu\":" "{\"micro\": 91, \"minor\": 13, \"major\": 0}," - "\"package\": \" (qemu-kvm-devel)\"}, \"capabilities\": []}}"); - + "\"package\": \" (qemu-kvm-devel)\"}, \"capabilities\": []}}", + "{\"QMP\":{\"version\":{\"qemu\":" + "{\"micro\":91,\"minor\":13,\"major\":0}," + "\"package\":\" (qemu-kvm-devel)\"},\"capabilities\":[]}}"); DO_TEST_PARSE("Harder", "{\"return\": [{\"filename\": " "\"unix:/home/berrange/.libvirt/qemu/lib/tck.monitor,server\"," "\"label\": \"charmonitor\"}, {\"filename\": \"pty:/dev/pts/158\"," - "\"label\": \"charserial0\"}], \"id\": \"libvirt-3\"}"); + "\"label\": \"charserial0\"}], \"id\": \"libvirt-3\"}", + "{\"return\":[{\"filename\":" + "\"unix:/home/berrange/.libvirt/qemu/lib/tck.monitor,server\"," + "\"label\":\"charmonitor\"},{\"filename\":\"pty:/dev/pts/158\"," + "\"label\":\"charserial0\"}],\"id\":\"libvirt-3\"}"); DO_TEST_PARSE("VeryHard", "{\"return\":[{\"name\":\"quit\"},{\"name\":" "\"eject\"},{\"name\":\"change\"},{\"name\":\"screendump\"}," @@ -364,7 +393,7 @@ mymain(void) "\"query-mice\"},{\"name\":\"query-vnc\"},{\"name\":" "\"query-spice\"},{\"name\":\"query-name\"},{\"name\":" "\"query-uuid\"},{\"name\":\"query-migrate\"},{\"name\":" - "\"query-balloon\"}],\"id\":\"libvirt-2\"}"); + "\"query-balloon\"}],\"id\":\"libvirt-2\"}", NULL); DO_TEST_FULL("add and remove", AddRemove, "{\"name\": \"sample\", \"value\": true}", @@ -400,21 +429,22 @@ mymain(void) "\"query-balloon\"}], \"id\": \"libvirt-2\"}", NULL, true); - DO_TEST_PARSE("almost nothing", "[]"); + DO_TEST_PARSE("almost nothing", "[]", NULL); DO_TEST_PARSE_FAIL("nothing", ""); - DO_TEST_PARSE("number without garbage", "[ 234545 ]"); + DO_TEST_PARSE("number without garbage", "[ 234545 ]", "[234545]"); DO_TEST_PARSE_FAIL("number with garbage", "[ 2345b45 ]"); - DO_TEST_PARSE("float without garbage", "[ 0.0314159e+100 ]"); + DO_TEST_PARSE("float without garbage", "[ 0.0314159e+100 ]", "[0.0314159e+100]"); DO_TEST_PARSE_FAIL("float with garbage", "[ 0.0314159ee+100 ]"); - DO_TEST_PARSE("string", "[ \"The meaning of life\" ]"); + DO_TEST_PARSE("string", "[ \"The meaning of life\" ]", + "[\"The meaning of life\"]"); DO_TEST_PARSE_FAIL("unterminated string", "[ \"The meaning of lif ]"); - DO_TEST_PARSE("integer", "1"); - DO_TEST_PARSE("boolean", "true"); - DO_TEST_PARSE("null", "null"); + DO_TEST_PARSE("integer", "1", NULL); + DO_TEST_PARSE("boolean", "true", NULL); + DO_TEST_PARSE("null", "null", NULL); DO_TEST_PARSE_FAIL("incomplete keyword", "tr"); DO_TEST_PARSE_FAIL("overdone keyword", "[ truest ]"); DO_TEST_PARSE_FAIL("unknown keyword", "huh"); -- 2.12.2

Make sure that JSON strings can contain characters which need to be escaped (double quotes, backslashes, tabs, etc.) and that JSON objects formatted into strings can be nested into strings. --- tests/virjsontest.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/virjsontest.c b/tests/virjsontest.c index 451275a4c..e4176af0e 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -325,6 +325,67 @@ testJSONCopy(const void *data) static int +testJSONEscapeObj(const void *data ATTRIBUTE_UNUSED) +{ + virJSONValuePtr json = NULL; + virJSONValuePtr nestjson = NULL; + virJSONValuePtr parsejson = NULL; + char *neststr = NULL; + char *result = NULL; + const char *parsednestedstr; + int ret = -1; + + if (virJSONValueObjectCreate(&nestjson, + "s:stringkey", "stringvalue", + "i:numberkey", 1234, + "b:booleankey", false, NULL) < 0) { + VIR_TEST_VERBOSE("failed to create nested json object"); + goto cleanup; + } + + if (!(neststr = virJSONValueToString(nestjson, false))) { + VIR_TEST_VERBOSE("failed to format nested json object"); + goto cleanup; + } + + if (virJSONValueObjectCreate(&json, "s:test", neststr, NULL) < 0) { + VIR_TEST_VERBOSE("Failed to create json object"); + goto cleanup; + } + + if (!(result = virJSONValueToString(json, false))) { + VIR_TEST_VERBOSE("Failed to format json object"); + goto cleanup; + } + + if (!(parsejson = virJSONValueFromString(result))) { + VIR_TEST_VERBOSE("Failed to parse JSON with nested JSON in string"); + goto cleanup; + } + + if (!(parsednestedstr = virJSONValueObjectGetString(parsejson, "test"))) { + VIR_TEST_VERBOSE("Failed to retrieve string containing nested json"); + goto cleanup; + } + + if (STRNEQ(parsednestedstr, neststr)) { + virTestDifference(stderr, neststr, parsednestedstr); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(neststr); + VIR_FREE(result); + virJSONValueFree(json); + virJSONValueFree(nestjson); + virJSONValueFree(parsejson); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -445,6 +506,10 @@ mymain(void) DO_TEST_PARSE("integer", "1", NULL); DO_TEST_PARSE("boolean", "true", NULL); DO_TEST_PARSE("null", "null", NULL); + + DO_TEST_PARSE("escaping symbols", "[\"\\\"\\t\\n\\\\\"]", NULL); + DO_TEST_PARSE("escaped strings", "[\"{\\\"blurb\\\":\\\"test\\\"}\"]", NULL); + DO_TEST_PARSE_FAIL("incomplete keyword", "tr"); DO_TEST_PARSE_FAIL("overdone keyword", "[ truest ]"); DO_TEST_PARSE_FAIL("unknown keyword", "huh"); @@ -477,6 +542,8 @@ mymain(void) DO_TEST_FULL("lookup with correct type", Lookup, "{ \"a\": {}, \"b\": 1, \"c\": \"str\", \"d\": [] }", NULL, true); + DO_TEST_FULL("create object with nested json in attribute", EscapeObj, + NULL, NULL, true); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.12.2

On Tue, Jul 04, 2017 at 12:53:23 +0200, Peter Krempa wrote:
For an experiment I was doing I needed to nest JSON string into an attribute of a different JSON string, so I wrote some tests for that.
The experiment failed, but the tests may make sense to have in libvirt.
This fails to apply now, so I'll post a rebased version soon.
participants (1)
-
Peter Krempa