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

[Daniel P. Berrangé]

On Thu, Oct 18, 2018 at 10:28:29AM +0200, Fabiano Fidêncio wrote:
> On Thu, 2018-10-18 at 09:07 +0100, Daniel P. Berrangé wrote:
> > 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
> > > 
> > > HmReviewed-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.
> 
> Daniel, I'd say at least test-mediauris and test-treeuris are basically
> the same case as the iso detect test and we should take a similar
> approach there than here.
> 
> So, just to be sure ... are you okay or not with having these 2 patches
> merged? Would you be okay on having a similar work done on media and
> tree uris tests?

Yes, its ok for these 3 tests

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