[Libosinfo] [osinfo-db-tools PATCH 0/7] Add tests for osinfo-db-tools

[Fabiano Fidêncio]

On Sun, Apr 7, 2019 at 8:46 PM Cole Robinson <crobinso at redhat.com> wrote:
>
> On 4/5/19 7:33 AM, Fabiano Fidêncio wrote:
> > This patchset introduces a bunch of functional tests for osinfo-db-tools,
> > covering all the tools and their command-lines.
> >
> > Daniel, what do you think we should do with regards to adding the python
> > dependencies to the spec file? Just add them as BuildRequires?
> >
>
> Yes that's how check deps are typically handled. It's a bit annoying but
> there isn't really any way around it.

Right. That was my understanding as well. However, there's no such
thing in libvirt-dbus spec file (well, they don't have the %check
target either).

I'll submit a follow-up patch including the dependencies.

>
> > This series has not been tested on EL7 at all. So, no python2 support is
> > provided unless someone has a strong opinion on adding it (then I'll find
> > out what has to be done in order to properly support it).
> >
>
> IMO just ignore py2 for now. We got this far without osinfo-db-tools
> tests, older platforms can continue on just fine, let's focus on the future.
>
> But please drop the last patch until you've done an RPM build test on
> centos/rhel7, otherwise I suspect CI is just going to break immediately
> due to 1) missing BuildRequires deps for all distros and 2) lack of
> conditionalizing on centos 7 where the deps aren't available.

CI will break but not because of the last patch :-)
As this series have been acked I'll submit the needed changes for
jenkins-ci and then we have this series tested there.

>
> > Also, we'll need to patch libvirt-jenkins-ci in order to update the deps
> > and ensure that make check is ran for osinfo-db-tools, but it's going to
> > be done after we have this series reviewed.
> >
> > https://gitlab.com/libosinfo/osinfo-db-tools/issues/2
> >
> > Fabiano Fidêncio (7):
> >   makefile: Add needed machinery for tests
> >   tests: Add util.py
> >   tests: Add tests data
> >   tests: Add osinfo-db-path tests
> >   tests: Add osinfo-db-validate tests
> >   tests: Add osinfo-db-{export,import} tests
> >   spec: Add %check target
> >
>
> I didn't go over the test content with a fine tooth comb or anything but
> it all looks + sounds reasonable to me, and it runs fine on my machine
> and on a freebsd vm. freebsd CI will need py36-requests and a few other
> python deps like we have for osinfo-db
>
> Reviewed-by: Cole Robinson <crobinso at redhat.com>
>
> But with the last patch dropped for now as mentioned above.
>
> Also we should add HACKING or CONTRIBUTING docs that mention the special
> environment variables for testing. We need the same for osinfo-db too

Right, follow-up patches will introduce those!

>
> Thanks,
> Cole

Thanks for the review, Cole!
Best Regards,
-- 
Fabiano Fidêncio