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

[Fabiano Fidêncio]

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?

> 
> Regards,
> Daniel