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