[ovs-dev] [RFC 00/55] Add Python 3 support.

Ben Pfaff blp at ovn.org
Tue Dec 22 00:34:07 UTC 2015


On Mon, Dec 21, 2015 at 07:29:41PM -0500, Russell Bryant wrote:
> On Mon, Dec 21, 2015 at 6:46 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > On Mon, Dec 21, 2015 at 06:20:42PM -0500, Russell Bryant wrote:
> > > On Mon, Dec 21, 2015 at 5:14 PM, Ben Pfaff <blp at ovn.org> wrote:
> > >
> > > > On Sat, Dec 19, 2015 at 11:37:00AM -0500, Russell Bryant wrote:
> > > > > $ make check TESTSUITEFLAGS="-k pep8"
> > > > >
> > > > > Another option would be to run this part at ovs build time, similar
> > to a
> > > > > number of other checks currently being done.
> > > > >
> > > > > $ grep 'ALL_LOCAL.*\-check' Makefile.am
> > > > > ALL_LOCAL += config-h-check
> > > > > ALL_LOCAL += printf-check
> > > > > ALL_LOCAL += static-check
> > > > > ALL_LOCAL += thread-safety-check
> > > > > ALL_LOCAL += manpage-check
> > > > >
> > > > > Here's what that would look like.  I don't have a strong opinion on
> > > > > whether it belongs just in 'make check' or at build time.
> > > >
> > > > How fast does it run?  If it's quick then I'd lean toward doing it at
> > > > build time.
> > >
> > > On my laptop, it takes a little over 4 seconds the first time and a
> > little
> > > over 2 seconds for any subsequent run.  The first time it has a little
> > more
> > > work to do to set up a virtual python environment.
> > >
> > > The patch I have for it makes it only run once and then again only if
> > > Python files change.  I can append it to v2 of this series if the time
> > > seems reasonable.
> >
> > OK.  Given that it runs only when Python files change, that sounds
> > reasonable.
> >
> 
> A downside I just thought of is that the way it's written requires internet
> access.  tox downloads packages from PyPI (the Python Package Index) to
> create the test environment.  That would probably be annoying sometimes if
> it was the only part of the build that requirement internet access.
> 
> The benefits of the way it works is that people don't have to install the
> test requirements on their system just to run tests.  In this case, that's
> just flake8, though.  The 'six' library dependency introduced in this patch
> series has to be installed on the system since the library is used by
> scripts in the build process.  It also ensures we're all using the same
> version of the dependencies.  The same internet access requirement also
> applies to the current patch that just runs it in 'make check'.
> 
> It's effectively only one dependency, though, so I could just change it so
> you have to install flake8 on your system if you want the checks to run.  I
> think I've just convinced myself that's better for this simple case.

OK.

We have other tests that run only if a particular program is installed
and available, so this fits the precedent.



More information about the dev mailing list