Hi Fabiano, On Mon, 2016-03-14 at 08:45 +0100, Fabiano Fidêncio wrote: > On Fri, Mar 11, 2016 at 6:13 PM, Pavel Grunt <pgrunt@xxxxxxxxxx> > wrote: > > > > Keep tests separated from the code > > --- > > v2: > > used space before '\' > > removed unneeded CFLAGS and LDADDs > > CFLAGS for gtk and glib are needed because virt-viewer-util > > includes gtk header > > --- [snip] > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > new file mode 100644 > > index 0000000..e2c9bba > > --- /dev/null > > +++ b/tests/Makefile.am > > @@ -0,0 +1,26 @@ > > +NULL = > > + > > +AM_CPPFLAGS = \ > > + -DLOCALE_DIR=\""$(datadir)/locale"\" \ > > + -I$(top_srcdir)/src/ \ > > + -I$(top_srcdir)/tests/ \ > > + $(GLIB2_CFLAGS) \ > > + $(GTK_CFLAGS) \I > I don't think we need GTK_CFLAGS. If we do, why? > CFLAGS for gtk and glib are needed because virt-viewer-util includes gtk header and our tests include glib header > And no need to add GLIB2_LIBS to the LDFLAGS? > It is linked thanks to libvirt-viewer-util.la Pavel > > > > + $(WARN_CFLAGS) \ > > + $(NULL) > > + > > +LDADD= \ > > + $(top_builddir)/src/libvirt-viewer-util.la \ > > + $(NULL) > > + > > +TESTS = test-version-compare test-monitor-mapping > > +check_PROGRAMS = $(TESTS) > > +test_version_compare_SOURCES = \ > > + test-version-compare.c \ > > + $(NULL) > > + > > +test_monitor_mapping_SOURCES = \ > > + test-monitor-mapping.c \ > > + $(NULL) > > + > > +-include $(top_srcdir)/git.mk > > diff --git a/tests/test-monitor-mapping.c b/tests/test-monitor- > > mapping.c > > new file mode 100644 > > index 0000000..004aa51 > > --- /dev/null > > +++ b/tests/test-monitor-mapping.c > > @@ -0,0 +1,119 @@ > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > > +/* > > + * Virt Viewer: A virtual machine console viewer > > + * > > + * Copyright (C) 2016 Red Hat, Inc. > > + * > > + * This program is free software; you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as > > published by > > + * the Free Software Foundation; either version 2 of the License, > > or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111- > > 1307 USA > > + */ > > + > > +#include <config.h> > > +#include <glib.h> > > +#include <virt-viewer-util.h> > > + > > +gboolean doDebug = FALSE; > > + > > +/** > > + * is_valid_monitor_mapping: > > + * @mapping: a value for the "monitor-mapping" key > > + * > > + * Tests the validity of the settings file for the "monitor- > > mapping" key: > > + * [test-monitor-mapping] > > + * monitor-mapping=@mapping > > + * > > + * Returns: %TRUE if the mapping is valid > > + */ > > +static gboolean > > +is_valid_monitor_mapping(const gchar *mapping) > > +{ > > + GKeyFile *key_file; > > + gboolean valid; > > + const gint nmonitors = 4; > > + const gchar *group_name = "test-monitor-mapping"; > > + const gchar *key = "monitor-mapping"; > > + const gchar *key_data_fmt = "[%s]\n%s=%s\n"; > > + gchar *key_data = g_strdup_printf(key_data_fmt, group_name, > > key, mapping); > > + > > + key_file = g_key_file_new(); > > + valid = g_key_file_load_from_data(key_file, key_data, -1, > > G_KEY_FILE_NONE, NULL); > > + if (valid) { > > + gsize nmappings; > > + gchar **mappings = g_key_file_get_string_list(key_file, > > group_name, key, &nmappings, NULL); > > + GHashTable *map = > > virt_viewer_parse_monitor_mappings(mappings, nmappings, nmonitors); > > + > > + valid = (map != NULL); > > + > > + g_strfreev(mappings); > > + g_clear_pointer(&map, g_hash_table_unref); > > + } > > + > > + g_key_file_free(key_file); > > + g_free(key_data); > > + return valid; > > +} > > + > > +int main(void) > > +{ > > + /* valid monitor mappings */ > > + g_assert_true(is_valid_monitor_mapping("1:1")); > > + g_assert_true(is_valid_monitor_mapping("1:1;2:2")); > > + g_assert_true(is_valid_monitor_mapping("1:1;2:2;3:3;")); > > + g_assert_true(is_valid_monitor_mapping("1:2;2:1;3:3;4:4")); > > + g_assert_true(is_valid_monitor_mapping("4:1;3:2;2:3;1:4")); > > + > > + /* invalid monitors mappings */ > > + /* zero ids */ > > + g_assert_false(is_valid_monitor_mapping("0:0")); > > + /* negative guest display id */ > > + g_assert_false(is_valid_monitor_mapping("-1:1")); > > + /* negative client monitor id */ > > + g_assert_false(is_valid_monitor_mapping("1:-1")); > > + /* negative guest display & client monitor id */ > > + g_assert_false(is_valid_monitor_mapping("-1:-1")); > > + /* high guest display id */ > > + g_assert_false(is_valid_monitor_mapping("100:1")); > > + /* high client monitor id */ > > + g_assert_false(is_valid_monitor_mapping("1:100")); > > + /* missing guest display id */ > > + g_assert_false(is_valid_monitor_mapping("1:1;3:3")); > > + /* guest display id used twice */ > > + g_assert_false(is_valid_monitor_mapping("1:1;1:2")); > > + /* client monitor id used twice */ > > + g_assert_false(is_valid_monitor_mapping("1:1;2:1")); > > + /* floating point guest display id */ > > + g_assert_false(is_valid_monitor_mapping("1.111:1")); > > + /* floating point client monitor id */ > > + g_assert_false(is_valid_monitor_mapping("1:1.111")); > > + /* empty mapping */ > > + g_assert_false(is_valid_monitor_mapping("")); > > + g_assert_false(is_valid_monitor_mapping(";")); > > + /* missing guest display id */ > > + g_assert_false(is_valid_monitor_mapping(":1")); > > + /* missing client monitor id */ > > + g_assert_false(is_valid_monitor_mapping("1:")); > > + /* missing guest display & client monitor id */ > > + g_assert_false(is_valid_monitor_mapping(":")); > > + /*missing colon */ > > + g_assert_false(is_valid_monitor_mapping("11")); > > + /*missing semicolon */ > > + g_assert_false(is_valid_monitor_mapping("1:12:2")); > > + /* strings */ > > + g_assert_false(is_valid_monitor_mapping("1:a")); > > + g_assert_false(is_valid_monitor_mapping("a:1")); > > + g_assert_false(is_valid_monitor_mapping("a:a")); > > + g_assert_false(is_valid_monitor_mapping("monitor mapping")); > > + > > + return 0; > > +} > > diff --git a/tests/test-version-compare.c b/tests/test-version- > > compare.c > > new file mode 100644 > > index 0000000..f14887b > > --- /dev/null > > +++ b/tests/test-version-compare.c > > @@ -0,0 +1,61 @@ > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > > +/* > > + * Virt Viewer: A virtual machine console viewer > > + * > > + * Copyright (C) 2015 Red Hat, Inc. > > + * > > + * This program is free software; you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as > > published by > > + * the Free Software Foundation; either version 2 of the License, > > or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111- > > 1307 USA > > + */ > > + > > +#include <config.h> > > +#include <glib.h> > > +#include <virt-viewer-util.h> > > + > > +gboolean doDebug = FALSE; > > + > > +int main(void) > > +{ > > + g_assert(virt_viewer_compare_buildid("1-1", "1-1") == 0); > > + g_assert(virt_viewer_compare_buildid("1-1", "1-1.1") < 0); > > + g_assert(virt_viewer_compare_buildid("1-1", "1-2") < 0); > > + g_assert(virt_viewer_compare_buildid("1-3", "1-2") > 0); > > + g_assert(virt_viewer_compare_buildid("2-3", "1-2") > 0); > > + g_assert(virt_viewer_compare_buildid("2-3", "3-2") < 0); > > + g_assert(virt_viewer_compare_buildid("2-3", "3-4") < 0); > > + g_assert(virt_viewer_compare_buildid("4-3", "3-4") > 0); > > + > > + g_assert(virt_viewer_compare_buildid("4.0-", "3-4") > 0); > > + g_assert(virt_viewer_compare_buildid("4.0-", "3.4-4") > 0); > > + g_assert(virt_viewer_compare_buildid(".0-", "3.4-4") < 0); > > + g_assert(virt_viewer_compare_buildid("4-", "3-4") > 0); > > + g_assert(virt_viewer_compare_buildid("4-3", "3-") > 0); > > + g_assert(virt_viewer_compare_buildid("-3", "3-4") < 0); > > + g_assert(virt_viewer_compare_buildid("4-3", "-4") > 0); > > + g_assert(virt_viewer_compare_buildid("-3", "-4") < 0); > > + g_assert(virt_viewer_compare_buildid("4", "3-4") > 0); > > + g_assert(virt_viewer_compare_buildid("4-3", "3") > 0); > > + g_assert(virt_viewer_compare_buildid("3", "3-4") < 0); > > + g_assert(virt_viewer_compare_buildid("4-3", "4") > 0); > > + g_assert(virt_viewer_compare_buildid("-3", "-4") < 0); > > + > > + /* These trigger runtime warnings */ > > + g_assert(virt_viewer_compare_buildid("-3", "-") > 0); > > + g_assert(virt_viewer_compare_buildid("", "-") == 0); > > + g_assert(virt_viewer_compare_buildid("", "") == 0); > > + g_assert(virt_viewer_compare_buildid("", NULL) == 0); > > + g_assert(virt_viewer_compare_buildid(NULL, NULL) == 0); > > + > > + return 0; > > +} > > -- > > 2.7.2 > > > > _______________________________________________ > > virt-tools-list mailing list > > virt-tools-list@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/virt-tools-list > You don't need the resend the patches. A small diff on top of this > one > is enough. > Best Regards, > -- > Fabiano Fidêncio _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list