Re: [Libosinfo] [libosinfo PATCH 3/3] README: document "check" for tests
On Tuesday, 17 October 2017 17:40:19 CEST Daniel P. Berrange wrote: > On Tue, Oct 17, 2017 at 05:23:33PM +0200, Pino Toscano wrote: > > "check" is required when building the tests (which are enabled by > > default), so document it. > > > > Signed-off-by: Pino Toscano > > --- > > README | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/README b/README > > index 1a0fa2e..11063ca 100644 > > --- a/README > > +++ b/README > > @@ -19,6 +19,7 @@ Dependencies > > > > > > - Required: > > + - check (for tests only) > >- gobject-2.0 > >- gio-2.0 > >- libxml-2.0 > > Reviewed-by: Daniel P. Berrange > > > Another future thing we should do is rewrite tests to use glib's native > unit test infrastructure and ditch 'check' entirely. Actually I withdraw this patch, since I'm going to send a conversion of the tests to the GLib test framework. Thanks, -- Pino Toscano signature.asc Description: This is a digitally signed message part. ___ Libosinfo mailing list Libosinfo@redhat.com https://www.redhat.com/mailman/listinfo/libosinfo
[Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
GLib has shipped a testing framework for many years already, so we can
make use of it, replacing the external "check".
The conversion only switches framework without changing the structure of
the tests, making use of the more appropriate assertion in the various
places.
Signed-off-by: Pino Toscano
---
configure.ac| 1 -
libosinfo.spec.in | 1 -
tests/Makefile.am | 3 +-
tests/test-db.c | 165 +
tests/test-device.c | 33 ++--
tests/test-devicelist.c | 77 +++--
tests/test-entity.c | 195
tests/test-filter.c | 111 +++--
tests/test-install-script.c | 92 -
tests/test-isodetect.c | 138 ++-
tests/test-list.c | 145 +++-
tests/test-loader.c | 33 ++--
tests/test-mediauris.c | 44 --
tests/test-os.c | 122 ---
tests/test-oslist.c | 77 +++--
tests/test-platform.c | 61 ++
tests/test-platformlist.c | 77 +++--
tests/test-product.c| 100 ++-
tests/test-productfilter.c | 143 +++-
tests/test-treeuris.c | 44 --
20 files changed, 694 insertions(+), 968 deletions(-)
diff --git a/configure.ac b/configure.ac
index 99abe94..d834c89 100644
--- a/configure.ac
+++ b/configure.ac
@@ -57,7 +57,6 @@ AC_ARG_ENABLE([tests],
if test "x$enable_tests" != "xno" ; then
have_curl=no
- PKG_CHECK_MODULES([CHECK], [check])
PKG_CHECK_MODULES([CURL], [libcurl], [have_curl=yes], [:])
AC_SUBST(CURL_CFLAGS)
AC_SUBST(CURL_LIBS)
diff --git a/libosinfo.spec.in b/libosinfo.spec.in
index 5a5e6f8..f3e3690 100644
--- a/libosinfo.spec.in
+++ b/libosinfo.spec.in
@@ -11,7 +11,6 @@ BuildRoot:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
URL: https://libosinfo.org/
BuildRequires: intltool
BuildRequires: glib2-devel
-BuildRequires: check-devel
BuildRequires: libxml2-devel >= 2.6.0
BuildRequires: libxslt-devel >= 1.0.0
BuildRequires: vala
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9315e04..7566d3c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -30,7 +30,6 @@ COMMON_LDADD = \
$(COVERAGE_LDFLAGS) \
$(GLIB_LIBS) \
$(GOBJECT_LIBS) \
- $(CHECK_LIBS) \
../osinfo/libosinfo-1.0.la
COMMON_CFLAGS = \
$(WARN_CFLAGS) \
@@ -40,7 +39,7 @@ COMMON_CFLAGS = \
-I$(top_srcdir) \
-DSRCDIR="\"$(abs_top_srcdir)\"" \
-DBUILDDIR="\"$(abs_top_builddir)\"" \
- $(CHECK_CFLAGS)
+ $(NULL)
test_entity_LDADD = $(COMMON_LDADD)
test_entity_CFLAGS = $(COMMON_CFLAGS)
diff --git a/tests/test-db.c b/tests/test-db.c
index 31e80d1..ac0a529 100644
--- a/tests/test-db.c
+++ b/tests/test-db.c
@@ -21,22 +21,21 @@
#include
-#include
#include
-#include
-START_TEST(test_basic)
+static void
+test_basic(void)
{
OsinfoDb *db = osinfo_db_new();
-fail_unless(OSINFO_IS_DB(db), "Db is not a DB");
+g_assert_true(OSINFO_IS_DB(db));
g_object_unref(db);
}
-END_TEST
-START_TEST(test_device)
+static void
+test_device(void)
{
OsinfoDb *db = osinfo_db_new();
OsinfoDevice *dev1 = osinfo_device_new("dev1");
@@ -48,24 +47,24 @@ START_TEST(test_device)
osinfo_db_add_device(db, dev3);
OsinfoDeviceList *list = osinfo_db_get_device_list(db);
-fail_unless(OSINFO_ENTITY(dev1) == osinfo_list_get_nth(OSINFO_LIST(list),
0), "Dev 1 is missing");
-fail_unless(OSINFO_ENTITY(dev2) == osinfo_list_get_nth(OSINFO_LIST(list),
1), "Dev 2 is missing");
-fail_unless(OSINFO_ENTITY(dev3) == osinfo_list_get_nth(OSINFO_LIST(list),
2), "Dev 3 is missing");
+g_assert_true(OSINFO_ENTITY(dev1) ==
osinfo_list_get_nth(OSINFO_LIST(list), 0));
+g_assert_true(OSINFO_ENTITY(dev2) ==
osinfo_list_get_nth(OSINFO_LIST(list), 1));
+g_assert_true(OSINFO_ENTITY(dev3) ==
osinfo_list_get_nth(OSINFO_LIST(list), 2));
g_object_unref(list);
OsinfoDevice *dev = osinfo_db_get_device(db, "dev2");
-fail_unless(dev != NULL, "Device is NULL");
-fail_unless(dev == dev2, "Device was not dev2");
+g_assert_nonnull(dev);
+g_assert_true(dev == dev2);
g_object_unref(dev1);
g_object_unref(dev2);
g_object_unref(dev3);
g_object_unref(db);
}
-END_TEST
-START_TEST(test_platform)
+static void
+test_platform(void)
{
OsinfoDb *db = osinfo_db_new();
OsinfoPlatform *hv1 = osinfo_platform_new("hv1");
@@ -77,24 +76,24 @@ START_TEST(test_platform)
osinfo_db_add_platform(db, hv3);
OsinfoPlatformList *list = osinfo_db_get_platform_list(db);
-fail_unless(OSINFO_ENTITY(hv1) == osinfo_list_get_nth(OSINFO_LIST(list),
0), "Hv 1 is missing");
-fail_unless(OSINFO_ENTITY(hv2) == osinfo_list_ge
Re: [Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
On Thursday, 19 October 2017 15:14:24 CEST Pino Toscano wrote: > GLib has shipped a testing framework for many years already, so we can > make use of it, replacing the external "check". > > The conversion only switches framework without changing the structure of > the tests, making use of the more appropriate assertion in the various > places. > > Signed-off-by: Pino Toscano > --- Meh, I suck, I accentally pushed it (see why I did not wanted powers!). Apologies. I can revert it right now, if needed. Otherwise, if there are just small edges to smooth, I can post cleanup patches. Let me know. Sorry again, -- Pino Toscano signature.asc Description: This is a digitally signed message part. ___ Libosinfo mailing list Libosinfo@redhat.com https://www.redhat.com/mailman/listinfo/libosinfo
Re: [Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
On Thu, Oct 19, 2017 at 03:33:33PM +0200, Pino Toscano wrote: > On Thursday, 19 October 2017 15:14:24 CEST Pino Toscano wrote: > > GLib has shipped a testing framework for many years already, so we can > > make use of it, replacing the external "check". > > > > The conversion only switches framework without changing the structure of > > the tests, making use of the more appropriate assertion in the various > > places. > > > > Signed-off-by: Pino Toscano > > --- > > Meh, I suck, I accentally pushed it (see why I did not wanted powers!). > Apologies. > > I can revert it right now, if needed. Otherwise, if there are just > small edges to smooth, I can post cleanup patches. Let me know. Its not a problem - it all looks fine to me - I just had one RFE suggestion Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ Libosinfo mailing list Libosinfo@redhat.com https://www.redhat.com/mailman/listinfo/libosinfo
Re: [Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
On Thu, Oct 19, 2017 at 03:14:24PM +0200, Pino Toscano wrote:
> GLib has shipped a testing framework for many years already, so we can
> make use of it, replacing the external "check".
>
> The conversion only switches framework without changing the structure of
> the tests, making use of the more appropriate assertion in the various
> places.
>
> Signed-off-by: Pino Toscano
> ---
> configure.ac| 1 -
> libosinfo.spec.in | 1 -
> tests/Makefile.am | 3 +-
> tests/test-db.c | 165 +
> tests/test-device.c | 33 ++--
> tests/test-devicelist.c | 77 +++--
> tests/test-entity.c | 195
>
> tests/test-filter.c | 111 +++--
> tests/test-install-script.c | 92 -
> tests/test-isodetect.c | 138 ++-
> tests/test-list.c | 145 +++-
> tests/test-loader.c | 33 ++--
> tests/test-mediauris.c | 44 --
> tests/test-os.c | 122 ---
> tests/test-oslist.c | 77 +++--
> tests/test-platform.c | 61 ++
> tests/test-platformlist.c | 77 +++--
> tests/test-product.c| 100 ++-
> tests/test-productfilter.c | 143 +++-
> tests/test-treeuris.c | 44 --
> 20 files changed, 694 insertions(+), 968 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 99abe94..d834c89 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -57,7 +57,6 @@ AC_ARG_ENABLE([tests],
>
> if test "x$enable_tests" != "xno" ; then
>have_curl=no
> - PKG_CHECK_MODULES([CHECK], [check])
>PKG_CHECK_MODULES([CURL], [libcurl], [have_curl=yes], [:])
>AC_SUBST(CURL_CFLAGS)
>AC_SUBST(CURL_LIBS)
> diff --git a/libosinfo.spec.in b/libosinfo.spec.in
> index 5a5e6f8..f3e3690 100644
> --- a/libosinfo.spec.in
> +++ b/libosinfo.spec.in
> @@ -11,7 +11,6 @@ BuildRoot:
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> URL: https://libosinfo.org/
> BuildRequires: intltool
> BuildRequires: glib2-devel
> -BuildRequires: check-devel
> BuildRequires: libxml2-devel >= 2.6.0
> BuildRequires: libxslt-devel >= 1.0.0
> BuildRequires: vala
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 9315e04..7566d3c 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -30,7 +30,6 @@ COMMON_LDADD = \
> $(COVERAGE_LDFLAGS) \
> $(GLIB_LIBS) \
> $(GOBJECT_LIBS) \
> - $(CHECK_LIBS) \
> ../osinfo/libosinfo-1.0.la
> COMMON_CFLAGS = \
> $(WARN_CFLAGS) \
> @@ -40,7 +39,7 @@ COMMON_CFLAGS = \
> -I$(top_srcdir) \
> -DSRCDIR="\"$(abs_top_srcdir)\"" \
> -DBUILDDIR="\"$(abs_top_builddir)\"" \
> - $(CHECK_CFLAGS)
> + $(NULL)
>
> test_entity_LDADD = $(COMMON_LDADD)
> test_entity_CFLAGS = $(COMMON_CFLAGS)
> diff --git a/tests/test-isodetect.c b/tests/test-isodetect.c
> index 7214531..97c86c4 100644
> --- a/tests/test-isodetect.c
> +++ b/tests/test-isodetect.c
> -int main(void)
> -{
> -int number_failed;
> -Suite *s = list_suite();
> -SRunner *sr = srunner_create(s);
> +g_test_init(&argc, &argv, NULL);
> +
> +g_test_add_func("/isodetect/fedora", test_fedora);
> +g_test_add_func("/isodetect/rhel", test_rhel);
> +g_test_add_func("/isodetect/ubuntu", test_ubuntu);
> +g_test_add_func("/isodetect/debian", test_debian);
> +g_test_add_func("/isodetect/windows", test_windows);
> +g_test_add_func("/isodetect/freebsd", test_freebsd);
> +g_test_add_func("/isodetect/openbsd", test_openbsd);
> +g_test_add_func("/isodetect/opensuse", test_opensuse);
> +g_test_add_func("/isodetect/centos", test_centos);
> +g_test_add_func("/isodetect/gnome", test_gnome);
> +g_test_add_func("/isodetect/altlinux", test_altlinux);
> +g_test_add_func("/isodetect/mageia", test_mageia);
> +g_test_add_func("/isodetect/sles", test_sles);
> +g_test_add_func("/isodetect/sled", test_sled);
We could wrap all of this in a
if (g_test_slow()) {
}
so that this only executes if people request slow tests via '-m slow' arg
Regardless, fine to push as is
Reviewed-by: Daniel P. Berrange
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo
Re: [Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
On Thursday, 19 October 2017 15:36:28 CEST Daniel P. Berrange wrote:
> On Thu, Oct 19, 2017 at 03:14:24PM +0200, Pino Toscano wrote:
> > diff --git a/tests/test-isodetect.c b/tests/test-isodetect.c
> > index 7214531..97c86c4 100644
> > --- a/tests/test-isodetect.c
> > +++ b/tests/test-isodetect.c
>
> > -int main(void)
> > -{
> > -int number_failed;
> > -Suite *s = list_suite();
> > -SRunner *sr = srunner_create(s);
> > +g_test_init(&argc, &argv, NULL);
> > +
> > +g_test_add_func("/isodetect/fedora", test_fedora);
> > +g_test_add_func("/isodetect/rhel", test_rhel);
> > +g_test_add_func("/isodetect/ubuntu", test_ubuntu);
> > +g_test_add_func("/isodetect/debian", test_debian);
> > +g_test_add_func("/isodetect/windows", test_windows);
> > +g_test_add_func("/isodetect/freebsd", test_freebsd);
> > +g_test_add_func("/isodetect/openbsd", test_openbsd);
> > +g_test_add_func("/isodetect/opensuse", test_opensuse);
> > +g_test_add_func("/isodetect/centos", test_centos);
> > +g_test_add_func("/isodetect/gnome", test_gnome);
> > +g_test_add_func("/isodetect/altlinux", test_altlinux);
> > +g_test_add_func("/isodetect/mageia", test_mageia);
> > +g_test_add_func("/isodetect/sles", test_sles);
> > +g_test_add_func("/isodetect/sled", test_sled);
>
> We could wrap all of this in a
>
>if (g_test_slow()) {
>
>}
>
> so that this only executes if people request slow tests via '-m slow' arg
Hm aren't these tests worth executing by default? Not sure what is the
proper definition of "slow" for GLib tests though, and test-isodetect
takes less than 20 seconds to run here.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo
Re: [Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
On Thu, Oct 19, 2017 at 03:59:02PM +0200, Pino Toscano wrote:
> On Thursday, 19 October 2017 15:36:28 CEST Daniel P. Berrange wrote:
> > On Thu, Oct 19, 2017 at 03:14:24PM +0200, Pino Toscano wrote:
> > > diff --git a/tests/test-isodetect.c b/tests/test-isodetect.c
> > > index 7214531..97c86c4 100644
> > > --- a/tests/test-isodetect.c
> > > +++ b/tests/test-isodetect.c
> >
> > > -int main(void)
> > > -{
> > > -int number_failed;
> > > -Suite *s = list_suite();
> > > -SRunner *sr = srunner_create(s);
> > > +g_test_init(&argc, &argv, NULL);
> > > +
> > > +g_test_add_func("/isodetect/fedora", test_fedora);
> > > +g_test_add_func("/isodetect/rhel", test_rhel);
> > > +g_test_add_func("/isodetect/ubuntu", test_ubuntu);
> > > +g_test_add_func("/isodetect/debian", test_debian);
> > > +g_test_add_func("/isodetect/windows", test_windows);
> > > +g_test_add_func("/isodetect/freebsd", test_freebsd);
> > > +g_test_add_func("/isodetect/openbsd", test_openbsd);
> > > +g_test_add_func("/isodetect/opensuse", test_opensuse);
> > > +g_test_add_func("/isodetect/centos", test_centos);
> > > +g_test_add_func("/isodetect/gnome", test_gnome);
> > > +g_test_add_func("/isodetect/altlinux", test_altlinux);
> > > +g_test_add_func("/isodetect/mageia", test_mageia);
> > > +g_test_add_func("/isodetect/sles", test_sles);
> > > +g_test_add_func("/isodetect/sled", test_sled);
> >
> > We could wrap all of this in a
> >
> >if (g_test_slow()) {
> >
> >}
> >
> > so that this only executes if people request slow tests via '-m slow' arg
>
> Hm aren't these tests worth executing by default? Not sure what is the
> proper definition of "slow" for GLib tests though, and test-isodetect
> takes less than 20 seconds to run here.
They're primarily useful for upstream to run - I don't think there's much
benefit downstream users running them. So I think its the kind of thing
where we could put '-m slow' into our CI system tests.
THis would avoid downstream users needing networking during tests too IIUC.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo
Re: [Libosinfo] [libosinfo PATCH] tests: convert from check to the GLib testing framework
On Thursday, 19 October 2017 16:02:18 CEST Daniel P. Berrange wrote:
> On Thu, Oct 19, 2017 at 03:59:02PM +0200, Pino Toscano wrote:
> > On Thursday, 19 October 2017 15:36:28 CEST Daniel P. Berrange wrote:
> > > On Thu, Oct 19, 2017 at 03:14:24PM +0200, Pino Toscano wrote:
> > > > diff --git a/tests/test-isodetect.c b/tests/test-isodetect.c
> > > > index 7214531..97c86c4 100644
> > > > --- a/tests/test-isodetect.c
> > > > +++ b/tests/test-isodetect.c
> > >
> > > > -int main(void)
> > > > -{
> > > > -int number_failed;
> > > > -Suite *s = list_suite();
> > > > -SRunner *sr = srunner_create(s);
> > > > +g_test_init(&argc, &argv, NULL);
> > > > +
> > > > +g_test_add_func("/isodetect/fedora", test_fedora);
> > > > +g_test_add_func("/isodetect/rhel", test_rhel);
> > > > +g_test_add_func("/isodetect/ubuntu", test_ubuntu);
> > > > +g_test_add_func("/isodetect/debian", test_debian);
> > > > +g_test_add_func("/isodetect/windows", test_windows);
> > > > +g_test_add_func("/isodetect/freebsd", test_freebsd);
> > > > +g_test_add_func("/isodetect/openbsd", test_openbsd);
> > > > +g_test_add_func("/isodetect/opensuse", test_opensuse);
> > > > +g_test_add_func("/isodetect/centos", test_centos);
> > > > +g_test_add_func("/isodetect/gnome", test_gnome);
> > > > +g_test_add_func("/isodetect/altlinux", test_altlinux);
> > > > +g_test_add_func("/isodetect/mageia", test_mageia);
> > > > +g_test_add_func("/isodetect/sles", test_sles);
> > > > +g_test_add_func("/isodetect/sled", test_sled);
> > >
> > > We could wrap all of this in a
> > >
> > >if (g_test_slow()) {
> > >
> > >}
> > >
> > > so that this only executes if people request slow tests via '-m slow' arg
> >
> > Hm aren't these tests worth executing by default? Not sure what is the
> > proper definition of "slow" for GLib tests though, and test-isodetect
> > takes less than 20 seconds to run here.
>
> They're primarily useful for upstream to run - I don't think there's much
> benefit downstream users running them. So I think its the kind of thing
> where we could put '-m slow' into our CI system tests.
Couldn't it be useful for downstream as well, i.e. to check that the
data provided in osinfo-db actually match the expected ISO data? Say
in case a downstream adds custom changes (or backport upstream commits),
so they know the modified osinfo-db is OK.
> THis would avoid downstream users needing networking during tests too IIUC.
Note that test-isodetect does not require networking, as it uses the
data in tests/isodata.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo
