On Fri, Mar 15, 2019 at 10:33 PM Cole Robinson <crobinso@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo
