[Libosinfo] [osinfo-db 2/2] test_media: Add detection test

[Fabiano Fidêncio]

On Fri, Mar 15, 2019 at 10:33 PM Cole Robinson <crobinso at redhat.com> wrote:
>
> On 3/15/19 11:51 AM, Fabiano Fidêncio wrote:
> > The detection test is similar to test-isodetect from libosinfo, apart
> > that it doesn't check for the filled-up languages in the matching media,
> > as we're not using libosinfo API to actually have a OsinfoMedia entity.
> >
> > A new dependency has been introduced, python3-regex, as the built-in
> > regex module for python doesn't cope with POSIX regex (which are used as
> > part of our db).
> >
>
> I suggest naming the test file after what it's functionally doing, not
> based on the object we are testing. So maybe test_isoinfo.py. Similarly
> move the isoinfo stuff out of generic named util.py and into its own
> file, probably just into test_isoinfo.py unless there's a clear
> different user on the horizon.
>
> > Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> > ---
> >  tests/unit/osinfo.py     | 40 +++++++++++++++++++
> >  tests/unit/test_media.py | 65 ++++++++++++++++++++++++++++---
> >  tests/unit/util.py       | 84 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 184 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/unit/osinfo.py b/tests/unit/osinfo.py
> > index b2d14c9..0b5e199 100644
> > --- a/tests/unit/osinfo.py
> > +++ b/tests/unit/osinfo.py
> > @@ -35,6 +35,11 @@ class Os():
> >          return shortid.text
> >      shortid = property(_get_shortid)
> >
> > +    def _get_distro(self):
> > +        distro = self._root.find('distro')
> > +        return distro.text
> > +    distro = property(_get_distro)
> > +
> >
> >  class Image():
> >      def __init__(self, root):
> > @@ -57,6 +62,41 @@ class Media():
> >              return URL(url.text)
> >      url = property(_get_url)
> >
> > +    def _get_iso(self):
> > +        iso = self._root.find('iso')
> > +        if iso is not None:
> > +            return ISO(iso)
> > +    iso = property(_get_iso)
> > +
> > +
> > +class ISO():
> > +    def __init__(self, root):
> > +        self._root = root
> > +
> > +    def _get_value(self, name, type=str, default=''):
> > +        entry = self._root.find(name)
> > +        return type(entry.text) if entry is not None else default
> > +
> > +    def _get_volumeid(self):
> > +        return self._get_value('volume-id')
> > +    volumeid = property(_get_volumeid)
> > +
> > +    def _get_publisherid(self):
> > +        return self._get_value('publisher-id')
> > +    publisherid = property(_get_publisherid)
> > +
> > +    def _get_applicationid(self):
> > +        return self._get_value('application-id')
> > +    applicationid = property(_get_applicationid)
> > +
> > +    def _get_systemid(self):
> > +        return self._get_value('system-id')
> > +    systemid = property(_get_systemid)
> > +
> > +    def _get_volumesize(self):
> > +        return self._get_value('volume-size', int, 0)
> > +    volumesize = property(_get_volumesize)
> > +
> >
> >  class Tree():
> >      def __init__(self, root):
> > diff --git a/tests/unit/test_media.py b/tests/unit/test_media.py
> > index f779aaa..890174e 100644
> > --- a/tests/unit/test_media.py
> > +++ b/tests/unit/test_media.py
> > @@ -2,19 +2,74 @@
> >
> >  import os
> >  import pytest
> > +import logging
> > +import regex
>
> Interesting, apparnetly regex is an alternate python regex library
> implementation. It's not from the standard library though, that's
> 'import re', which you can probably just sed replace at it will work
> just as well.

Kind of. The built-in 're' can't cope with POSIX patterns as
[:alpha:], [:upper:], [:digit:] and I didn't want to invest some time
doing before having at least some feedback about the series.

I'll change the ~20 entries that contain those patterns, and re-submit
then together with v2.


>
> Otherwise this looks fine, most important thing is that it works. I
> recommend getting a pylint setup though, it will point out certain
> things like not using 'type' as a variable name because it overwrites
> the global 'type' function in the local scope
>
> - Cole