Re: [osinfo-db PATCH 2/2] tests: add a new test for dates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2019-05-21 at 14:30 +0200, Fabiano Fidêncio wrote:
> On Tue, 2019-05-21 at 14:01 +0200, Pino Toscano wrote:
> > For each OS, check two things:
> > - release-date/eol-date are actually valid dates; this is needed
> > because
> >   the schema just specifies the regex, and it cannot detect invalid
> >   dates such as "2019-05-00" or 2019-05-40"
> > - if both release-date/eol-date are specified, eol-date must be
> > later
> >   than release-date
> > ---
> >  tests/test_dates.py | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >  create mode 100644 tests/test_dates.py
> > 
> > diff --git a/tests/test_dates.py b/tests/test_dates.py
> > new file mode 100644
> > index 0000000..60f70e6
> > --- /dev/null
> > +++ b/tests/test_dates.py
> > @@ -0,0 +1,22 @@
> > +# This work is licensed under the GNU GPLv2 or later.
> > +# See the COPYING file in the top-level directory.
> > +
> > +import datetime
> > +
> > +import pytest
> 
> There's no need to import pytest here.
> 
> > +
> > +from . import util
> > +
> > +
> > +def _parse_date(date_string):
> > +    if not date_string:
> > +        return None
> > +    return datetime.date.fromisoformat(date_string)
> 
> With the change suggested in the first patch, you can just ditch this
> method ...

You actually can't. Sorry. :-)
 
> 
> > +
> > +^
> > +@pytest.mark.parametrize('os', util.DataFiles.oses())
> 
> Here, I'd suggest to use `@util.os_parametrize('osxml',
> filter_dates=True)` and then un util.py just add a new filter_dates
> that would be something like:
> oses = [o for o in oses if o.release_date and o.eol_date]

The check to be done should be "or" as your original test was catching
single entries as well (as in just with release-date or just with eol-
date).

> 
> This will make the test to run only against the entries that have
> release and eol date, instead of running against every entry we have.
> 
> Of course, by doing this you'll have to adjust os -> osxml.

This comment still aplies.

> 
> 
> > +def test_dates(os):
> > +    release_date = _parse_date(os.release_date)
> > +    eol_date = _parse_date(os.eol_date)
> 
> ... and just call datetime.data.fromisoformat() directly here

This, doesn't. Sorry. :-)

> 
> > +    if release_date and eol_date:
> > +        assert release_date < eol_date


So, in the end, my suggestion is to add to this patches something like:
```
fidencio@laerte ~/src/upstream/osinfo-db $ git diff
diff --git a/tests/test_dates.py b/tests/test_dates.py
index 60f70e6..aa8db17 100644
--- a/tests/test_dates.py
+++ b/tests/test_dates.py
@@ -3,8 +3,6 @@
 
 import datetime
 
-import pytest
-
 from . import util
 
 
@@ -14,9 +12,9 @@ def _parse_date(date_string):
     return datetime.date.fromisoformat(date_string)
 
 
-@pytest.mark.parametrize('os', util.DataFiles.oses())
-def test_dates(os):
-    release_date = _parse_date(os.release_date)
-    eol_date = _parse_date(os.eol_date)
+@util.os_parametrize('osxml', filter_dates=True)
+def test_dates(osxml):
+    release_date = _parse_date(osxml.release_date)
+    eol_date = _parse_date(osxml.eol_date)
     if release_date and eol_date:
         assert release_date < eol_date
diff --git a/tests/util.py b/tests/util.py
index 19728a4..b5dcd8b 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -84,6 +84,8 @@ class _Files():
             oses = [o for o in oses if o.devices]
         if filter_resources:
             oses = [o for o in oses if o.resources_list]
+        if filter_dates:
+            oses = [o for o in oses if o.release_date or o.eol_date]
         return oses
 
     def getosxml_related(self, osxml):
```

If I get your ack on this, I can just squash the patch into yours and
push without the need of a v2.

Best Regards,
-- 
Fabiano Fidêncio

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux