On Mon, Mar 14, 2016 at 11:07 AM, Pavel Grunt <pgrunt@xxxxxxxxxx> wrote:
> On Mon, 2016-03-14 at 09:55 +0100, Pavel Grunt wrote:
>> On Mon, 2016-03-14 at 09:25 +0100, Fabiano Fidêncio wrote:
>> >
>> > On Mon, Mar 14, 2016 at 9:14 AM, Pavel Grunt <pgrunt@xxxxxxxxxx>
>> > wrote:
>> > >
>> > >
>> > > 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
>> > Ok, ACK them.
>> >
>> > >
>> > >
>> > >
>> > >
>> > > >
>> > > >
>> > > > And no need to add GLIB2_LIBS to the LDFLAGS?
>> > > >
>> > > It is linked thanks to libvirt-viewer-util.la
>> my bad, you are right, for some reason it is needed when running
>> tests
>> with wine. I will update the patch.
>>
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index e2c9bba..15907eb 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -11,6 +11,9 @@ AM_CPPFLAGS = \
>
> LDADD= \
> $(top_builddir)/src/libvirt-viewer-util.la \
> + $(GLIB2_LIBS) \
> + $(GTK_LIBS) \
> + $(LIBXML2_LIBS) \
> $(NULL)
>
> TESTS = test-version-compare test-monitor-mapping
ACK both patches, go ahead and pushed them (if it's not done already).
>
>
>
>
>> >
>> > >
>> > >
>> > > 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
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list
--
Fabiano Fidêncio
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
