flake8 not running consistently against all files

Bug #1208584 reported by Eric Harney on 2013-08-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Low
Kui Shi

Bug Description

Files in bin/ aren't being checked with flake8 in some scenarios.

<edit bin/cinder-volume to not pass flake8 tests>

$ flake8 bin/cinder-volume
bin/cinder-volume:62:47: E127 continuation line over-indented for visual indent
bin/cinder-volume:70:1: W391 blank line at end of file

$ ./run_tests.sh -N -p
Running flake8 ...
$

$ ./run_tests.sh -p
Running flake8 ...
./cinder/tests/test_coraid.py:286:10: H202 assertRaises Exception too broad
./cinder/tests/test_image_utils.py:189:10: H202 assertRaises Exception too broad
$

It looks like a) bin/cinder-* is not being checked, and b) rules aren't being applied consistently for cinder/tests/.

Eric Harney (eharney) on 2013-08-05
Changed in cinder:
importance: Undecided → Low
Kui Shi (skuicloud) on 2013-08-20
Changed in cinder:
assignee: nobody → Kui Shi (skuicloud)
Kui Shi (skuicloud) wrote :

$ ./run_tests.sh -N -p
this command will use the "flake8" program installed in our host.

$ ./run_tests.sh -p
this command will use the "flake8" installed in the virtualenv.

The two version of flake8 may be different. so the result may be different.

Kui Shi (skuicloud) wrote :

We should trust the result of "./run_tests.sh -p", which is same as the command used by jenkins job.

Kui Shi (skuicloud) wrote :

I run flake8 testing, there is still error.

$ ./run_tests.sh -p
Running flake8 ...
cinder/tests/test_image_utils.py:189:10: H202 assertRaises Exception too broad

For this bug, we should just fix the flake8 H202 error.

Kui Shi (skuicloud) wrote :

flake8 read the section [flake8] of tox.ini to control it behavior. the directory been checked are designated in the run_tests.sh.
if you use tox, the directory is designated in section [testenv:pep8] of tox.ini.

The checking directory are always determined and consistent.

Consider the title and content of this bug, I change the status to "Invalid".
I will open another bug to fix the H202 error.

Changed in cinder:
status: New → Invalid
Kui Shi (skuicloud) wrote :

The root cause is that H202 is detected by OpenStack Hacking only, which is triggered by flake8. The python package "hacking" is installed in virtualenv. so "run_tests.sh -p" can detect the H202 error.

But on your host, hacking is not installed by default, then flake8 will not detect such kind of error.

Eric Harney (eharney) wrote :

Good to know... IMO that's a bug in itself though.

This behavior would make sense if I was just running "flake8" but run_tests.sh is included with the project and should figure this kind of thing out itself. run_tests.sh is expected to run the same tests whether you use -N or -V.

We should at least print a warning message saying "your code is possibly not being checked against all rules" if run_tests.sh does anything differently from what the gate tests will do.

Changed in cinder:
status: Invalid → New
Kui Shi (skuicloud) wrote :

It is reasonable to change back to new.

I will add some logical clause to emit warning for the case of "run_tests.sh -N -p".

Fix proposed to branch: master
Review: https://review.openstack.org/43098

Changed in cinder:
status: New → In Progress

Reviewed: https://review.openstack.org/43098
Committed: http://github.com/openstack/cinder/commit/8619b974d17a186dc1013bfdc9d95f6a2e823335
Submitter: Jenkins
Branch: master

commit 8619b974d17a186dc1013bfdc9d95f6a2e823335
Author: Kui Shi <email address hidden>
Date: Wed Aug 21 19:07:43 2013 +0800

    emit warning while running flake8 without virtual env

    run_tests.sh -N -p
    it will call the flake8 installed on your host to detect PEP8, and
    the flake8 plugin "OpenStack hacking" may not installed on your
    host, so this command may not detect the OpenStack Style Commandment
    supplied by hacking(e.g H202).

    run_tests.sh -p
    it will call the flake8 from virtual env, flake8 plugin "OpenStack
    hacking" installed in virtual env will be triggered.

    The result from "run_tests.sh -p" should be trusted, and jenkins uses
    virtual env to run flake8 too.

    When "-N" is enabled, emit warning to remind user.

    Bug #1208584

    Change-Id: Ie08e5fa2b44088dad135e11583c046650d65df1e

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2013-09-05
Changed in cinder:
milestone: none → havana-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-17
Changed in cinder:
milestone: havana-3 → 2013.2
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers