[Libosinfo] [PATCH 2/2] test-isodetect: continue after failure

[Daniel P. Berrangé]

On Wed, Oct 17, 2018 at 09:12:53PM +0200, Fabiano Fidêncio wrote:
> On Wed, Oct 17, 2018 at 8:11 PM Věra Cholasta <vbudikov at redhat.com> wrote:
> >
> > ---
> >  configure.ac           | 4 ++--
> >  tests/test-isodetect.c | 7 +++++--
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 2e62955..00547a5 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -37,8 +37,8 @@ m4_if(m4_version_compare([2.61a.100],
> >  m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
> >
> >  # Keep these two definitions in agreement.
> > -GLIB_MINIMUM_VERSION="2.36"
> > -GLIB_ENCODED_VERSION="GLIB_VERSION_2_36"
> > +GLIB_MINIMUM_VERSION="2.38"
> > +GLIB_ENCODED_VERSION="GLIB_VERSION_2_38"
> >
> >  PKG_CHECK_MODULES([LIBXML], [libxml-2.0 >= 2.6.0])
> >  PKG_CHECK_MODULES([LIBXSLT], [libxslt >= 1.0.0])
> > diff --git a/tests/test-isodetect.c b/tests/test-isodetect.c
> > index c1833ae..6a4a2ba 100644
> > --- a/tests/test-isodetect.c
> > +++ b/tests/test-isodetect.c
> > @@ -398,8 +398,10 @@ static void test_one(const gchar *vendor)
> >          g_test_message("checking OS %s for ISO %s",
> >                         info->shortid, info->filename);
> >          if (!matched) {
> > -            g_error("ISO %s was not matched by OS %s",
> > -                    info->filename, info->shortid);
> > +            g_printerr("ISO %s was not matched by OS %s\n/isodetect/%s: ",
> > +                       info->filename, info->shortid, vendor);
> > +            g_test_fail();
> > +            continue;
> >          }
> >
> >          g_object_get(info->media, "os", &os, NULL);
> > @@ -419,6 +421,7 @@ int
> >  main(int argc, char *argv[])
> >  {
> >      g_test_init(&argc, &argv, NULL);
> > +    g_test_set_nonfatal_assertions();
> >
> >      GList *vendors = load_vendors(NULL);
> >      GList *it;
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > Libosinfo mailing list
> > Libosinfo at redhat.com
> > https://www.redhat.com/mailman/listinfo/libosinfo
> 
> Reviewed-by: Fabiano Fidêncio <fidencio at redhat.com>
> 
> Věra,
> 
> I see those 2 patches as the first step of a bigger work, as Cole
> mentioned in the bugzilla you've opened about this issue.
> 
> AFAIU, pretty much all other tests would benefit of similar changes.
> Would you be willing to work on that?

I'm not really in favour of that. Use of the g_assert() model with
immediate abort of the test is a good thing as it keeps the tests
clear & simple. If the asserts don't abort then in most cases you'll
get a cascade of failures unless you add extra logic to return
early from the test case method.

The iso detect test is special because failure of one match doesn't
have any bearing on  failure of the next match.

In fact arguably instead of having grouped the ISOs into 7 test
case based on OS distro, we should have just registered one test
case per ISO we need to test.

Then you could just pass the -k argument to the test case to have
it ignore failures after each test.

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 :|