sort out pyflakes flake8 pycodestyle pep8

Bug #1652329 reported by Scott Moser
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
cloud-init
Fix Released
Undecided
Unassigned
curtin
Fix Released
Undecided
Unassigned

Bug Description

I was again bit by this... tox works fine (running flake8)
but built of zesty failed.

In cloud-init, the Makefile that is run during build runs (currently) pep8 and pyflakes (either pyflakes2 or pyflakes3).

I think what we ultimately want is to run flake8 and let it run pyflakes and pep8.

However, given dependencies and versions in different distro releases, this is a bit of a pain.

One path to this is just to not run any code checking in the distro build, assuming that it is already run in trunk. I think that seems like a sane argument actually.

One thing to note, is that pep8 is now called pycodestyle.

Out of this, I'd like to have:
 a.) a tox target that tracks upstream pypi versions (but doesn't fail c-i), and somethign that runs this fairly often to at least see how bad we are getting.
 b.) a tox target that has pinned versions of checking tools, and runs checks both for python2 and python3.

Revision history for this message
Barry Warsaw (barry) wrote :

I've struggled a bit to get all this sorted out in ubuntu-image, but I have a workable solution, at least for X, Y, and Z. I only use flake8, with `sitepackages=True` and a few little tricks and plugins to get it all passing. One other thing I did with pitti before he left was to hook up GH CI to autopkgtest, so the tests are all running on the ubuntu ci machinery, exactly as it would for -proposed promotion. That includes package build times and DEP-8 tests.

Check out ubuntu-image on GH and happy to chat about it more after the holidays. I think it would make a lot of sense, if possible, for these three projects to have some consistent CI set up (although our coding styles may be minorly different).

Revision history for this message
Joshua Powers (powersj) wrote :

@smoser, before the holiday you mentioned how we have two targets: unit tests and style. For unit tests you mention wanting to move to flake8 and have runs for each Ubuntu release. For the unit tests, you still want these to gate the build. So why not break them out as follows:

tox.ini - This contains only the unit tests as well as the doc tests. The unit tests would be invoked during a build to essentially run nosetests on py27 and py3. See [1] for an example.

tox-style.ini - This contains the lint tests where flake8 is locked in at a specific version, whether that be for trusty, xenial, or the latest version found in pypi. Because we are not packaging up or running the code there is no need for any additional requirements above a specific version of flake8. This file would only be invoked by our CI system, by developers, and not during a build. See [2] for an example.

[1] https://paste.ubuntu.com/23741424/
[2] https://paste.ubuntu.com/23741427/

Revision history for this message
Barry Warsaw (barry) wrote : Re: [Bug 1652329] Re: sort out pyflakes flake8 pycodestyle pep8

On Jan 04, 2017, at 09:11 PM, Joshua Powers wrote:

>@smoser, before the holiday you mentioned how we have two targets: unit
>tests and style. For unit tests you mention wanting to move to flake8
>and have runs for each Ubuntu release. For the unit tests, you still
>want these to gate the build. So why not break them out as follows:

I don't think you *have* to have two separate tox.ini files for this. I
personally find it clearer to have a single tox.ini file. You don't have to
name all your environments in the default [tox]envlist; that's just the set
that gets run automatically when you don't provide an explicit set of
environments to run (e.g. through -e or $TOXENV.

The way I generally do this is to set up [testenv:FOO] sections for all the
tests I want, including various specifications for the generated environments,
but then I only name in envlist the ones I want to run by default during
development. Then in my CI file (e.g. .travis-ci.yml or whatever) I
explicitly invoke the environments I want to run for CI gating.

I don't have a great example of how this all hangs together with GitHub (since
e.g. ubuntu-image uses Ubuntu's autopkgtest infrastructure instead of Travis),
but for a GitLab example, see:

https://gitlab.com/mailman/mailman/blob/master/.gitlab-ci.yml

I'll just mention one other thing; this would all be much easier with these
two tox wishlists:

https://github.com/tox-dev/tox/issues/238

This would allow you to specify env groups so e.g. you could do something
like:

$ tox -g qa

and that would run the flake8 and any other style tests, while something like

$ tox -g ci

would run all the envs that you'd gate CI on.

https://github.com/tox-dev/tox/issues/418

Makes life easier for generated environments.

I certainly encourage more +1s on those issues if you agree with me! :)

Revision history for this message
Joshua Powers (powersj) wrote :

Hey Barry!

Sorry I didn't see this right away, forgot to subscribe to comments.

> I don't think you *have* to have two separate tox.ini files for this.

There were two reasons why I thought about doing it that way 1) to help me understand what smoser wants to split out and 2) so we could call them separately using:

$ tox
and
$ tox -c ci.ini

This sort of like the group idea you proposed, which I really, really like! That would be super handy at doing different types of tests and keeping things together. +1s given :)

> https://gitlab.com/mailman/mailman/blob/master/.gitlab-ci.yml

That is nice and makes me realize we could just use the Makefile to create custom targets for testing the different scenarios. When developing this I tried to do everything with just tox.

At this point I think I need smoser and I to sit down and go through the specifics of what he wants and where and then with your hints put it all together. Much appreciated!

Revision history for this message
Barry Warsaw (barry) wrote :

On Jan 06, 2017, at 08:31 PM, Joshua Powers wrote:

>There were two reasons why I thought about doing it that way 1) to help
>me understand what smoser wants to split out and 2) so we could call
>them separately using:
>
>$ tox
>and
>$ tox -c ci.ini
>
>This sort of like the group idea you proposed, which I really, really
>like! That would be super handy at doing different types of tests and
>keeping things together. +1s given :)

Awesome! Yeah, I might finally get motivated to contribute this feature to
upstream tox. I just keep wanting it more and more. :)

>> https://gitlab.com/mailman/mailman/blob/master/.gitlab-ci.yml
>
>That is nice and makes me realize we could just use the Makefile to
>create custom targets for testing the different scenarios. When
>developing this I tried to do everything with just tox.

Oh, I totally agree. I really want to do everything with tox. Say No To
Makefiles! :)

>At this point I think I need smoser and I to sit down and go through the
>specifics of what he wants and where and then with your hints put it all
>together. Much appreciated!

+1

Revision history for this message
Joshua Powers (powersj) wrote :
Changed in cloud-init:
status: New → Fix Released
Changed in curtin:
status: New → Fix Released
Revision history for this message
James Falcon (falcojr) wrote :
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.