[libvirt] [PATCH v2 0/4] tests: json: Improve testing of parsing and formatting
Now rebased to master after changes to the virjsontest. Purpose of this series was to test json nested in json. While it won't be used, it may be worth to test that our parser and formatter handles escaping/nesting properly. Peter Krempa (4): 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/virjsontest.c | 194 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 145 insertions(+), 49 deletions(-) -- 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 b9c210620..a5ffc4707 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -27,24 +27,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
On 07/11/2017 08:56 AM, Peter Krempa wrote:
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(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
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 a5ffc4707..029a580f4 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -386,30 +386,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
On 07/11/2017 08:56 AM, Peter Krempa wrote:
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(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
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 029a580f4..a6e158179 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -23,6 +23,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); @@ -45,9 +47,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; } @@ -368,23 +381,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\"}," @@ -409,7 +438,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}", @@ -445,21 +474,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
On 07/11/2017 08:56 AM, Peter Krempa wrote:
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(-)
Is there a reason why the Copy doesn't need the space reduction? Is that expected (so to speak). What's here looks fine, I'm mainly curious. Reviewed-by: John Ferlan <jferlan@redhat.com> John
On Wed, Jul 19, 2017 at 08:36:27 -0400, John Ferlan wrote:
On 07/11/2017 08:56 AM, Peter Krempa wrote:
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(-)
Is there a reason why the Copy doesn't need the space reduction? Is that expected (so to speak).
You mean the cases in testJSONCopy? It's because the test case parses the JSON provided when calling the test case, then copies it and then formats back strings from the first parsed structure and after the copy. This means that both went through the formatter and thus have same spacing rules applied. The output of formatting the copy is never compared to the original string provided when calling the test case.
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 | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/virjsontest.c b/tests/virjsontest.c index a6e158179..30457d118 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -365,7 +365,67 @@ testJSONDeflatten(const void *data) VIR_FREE(actual); return ret; +} + + +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; } @@ -490,6 +550,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"); @@ -522,6 +586,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); #define DO_TEST_DEFLATTEN(name, pass) \ DO_TEST_FULL(name, Deflatten, name, NULL, pass) -- 2.12.2
On 07/11/2017 08:56 AM, Peter Krempa wrote:
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 | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
Seems there should be two separate patches here, one that ensures escaped strings work for DO_TEST_PARSE and the other passing/using a nested string obj. Is "EscapeObj" even a right name, seems like "NestedObj" would be more appropriate. Is there some sort of escaping in the strings that I'm missing? Also, w/r/t escaping do we need handle double commas ",," that are supposed to turn into "," ? John
diff --git a/tests/virjsontest.c b/tests/virjsontest.c index a6e158179..30457d118 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -365,7 +365,67 @@ testJSONDeflatten(const void *data) VIR_FREE(actual);
return ret; +} + + +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; }
@@ -490,6 +550,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"); @@ -522,6 +586,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);
#define DO_TEST_DEFLATTEN(name, pass) \ DO_TEST_FULL(name, Deflatten, name, NULL, pass)
On Wed, Jul 19, 2017 at 09:08:16 -0400, John Ferlan wrote:
On 07/11/2017 08:56 AM, Peter Krempa wrote:
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 | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
Seems there should be two separate patches here, one that ensures escaped strings work for DO_TEST_PARSE and the other passing/using a
Okay I'll split it and resend.
nested string obj. Is "EscapeObj" even a right name, seems like "NestedObj" would be more appropriate. Is there some sort of escaping in the strings that I'm missing?
The naming issue is somewhere between those two, since you can nest JSON naturally, but nesting a json string in a json value is what we are tesing here.
Also, w/r/t escaping do we need handle double commas ",," that are supposed to turn into "," ?
Commas? Aren't you confusing this with escaping for qemu? JSON escaping rules don't have anything special for commas since the escaping needs to be done only if you want to store a JSON string into another JSON string. Commas are not interpreted inside of strings.
participants (2)
-
John Ferlan -
Peter Krempa