Re: [libvirt] [PATCH v2] [glib 5/5] gconfig, tests: Add test case for unknown devices

On Thu, Nov 03, 2016 at 02:05:46PM +0100, Zeeshan Ali wrote: > On Thu, Nov 3, 2016 at 1:03 PM, Christophe Fergeau <cfergeau(a)redhat.com&gt; wrote: > > On Thu, Nov 03, 2016 at 10:06:22AM +0100, Zeeshan Ali wrote: > >> --- > >> tests/test-gconfig.c | 22 ++++++++++++++++++++++ > >> tests/xml/gconfig-domain-device-unknown.xml | 5 +++++ > >> 2 files changed, 27 insertions(+) > >> create mode 100644 tests/xml/gconfig-domain-device-unknown.xml > >> > >> diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c > >> index 5389a26..b91f5af 100644 > >> --- a/tests/test-gconfig.c > >> +++ b/tests/test-gconfig.c > >> @@ -762,6 +762,26 @@ static void test_domain_device_pci_hostdev(void) > >> g_object_unref(G_OBJECT(domain)); > >> } > >> > >> +static void test_domain_device_unknown(void) > >> +{ > >> + GVirConfigDomain *domain; > >> + GList *devices; > >> + GError *error = NULL; > >> + char *xml; > >> + > >> + xml = load_xml("gconfig-domain-device-unknown.xml"); > >> + > >> + domain = gvir_config_domain_new_from_xml(xml, &error); > >> + g_assert_no_error(error); > >> + > >> + devices = gvir_config_domain_get_devices(domain); > >> + g_assert_nonnull(devices); > >> + > >> + g_list_free(devices); > >> + g_object_unref(G_OBJECT(domain)); > >> +} > > > > The whole point of this work is to make sure that the unknown nodes are > > kept when regenerating the XML document, so I would expect a call to > > check_xml() too after calling set_devices(domain, devices); > > * g_assert_nonnull(devices); actually checks that the only dev in the > original xml is still there or not. It checks that a device is there, it does say nothing about how it's going to be serialized, so the test case could be working while what you actually need is broken. > * check_xml() does a simple string comparison and that fails since > set_devices() changes the format. Isn't it possible to use the exact same format as the output for the source XML?