gluster / project-infrastructure

Issues related to GlusterFs infrastructure components.
0 stars 0 forks source link

[bug:1584992] Need python pep8 and other relevant tests in smoke if a patch includes any python file #29

Open gluster-ant opened 4 years ago

gluster-ant commented 4 years ago

URL: https://bugzilla.redhat.com/1584992 Creator: atumball at redhat Time: 20180601T06:18:40

Description of problem:

There are significant amount of python files in the codebase. There are very good python validation tools to check if the file is all fine.

Also, to make sure the code getting in is python3 compatible, we can automate the tests.

It would be good to run automated pep8 tests and pylint type of tests which can make reviewing much easier on reviewer.

Additional info:

This job should be able to vote -1 or +1 in smoke.

gluster-ant commented 4 years ago

Time: 20180601T09:59:31 nigelb at redhat commented: Okay, so this can be split into multiple pieces

All of these tests can only be voting jobs if we can show that they pass now. Are the current maintainers of these modules willing to spend some time fixing up all the errors in the next few weeks?

gluster-ant commented 4 years ago

Time: 20180601T10:26:47 nigelb at redhat commented: pep8 issues: 3K pylint issues: 12K pylint python3 issues: 370

It's not worth running a CI unless we can fix them or tweak the config so we care about the right things.

gluster-ant commented 4 years ago

Time: 20180608T13:01:28 avishwan at redhat commented: pep8 issues can be fixed by using autopep8 tool, I will work on it and send patch.

pylint gives lot of warnings related to variable naming style, we can ignore those errors. Also pylint errors are mostly about best practices and code suggestions. I feel we should defer pylint voting since it involves lot of code changes.

We can use flake8 for now to validate pep8, unused variables, imports etc.

https://github.com/gluster/libgfapi-python/blob/master/tox.ini

gluster-ant commented 4 years ago

Time: 20180703T09:03:09 nigelb at redhat commented: So re: python3 validation. I ran pylint --py3k across the codebase today and it showed me zero errors. However, running 2to3 for our python files shows me a lot of dict.iteritems() which should be dict.items() in python3 (Kaleb has a patch to fix them already). This still doesn't catch more subtler bugs like python's division operator changing behavior in python3

python2 5 / 2 = 2

python3 5 / 2 = 2.5

2to3 doesn't catch this change. In essence, we can add automation, but we warned that the automation passing does not actually mean that we're 100% in the clear with regard to python3 support. That will need some in-depth testing and there's no way around that.

gluster-ant commented 4 years ago

Time: 20181005T08:42:45 nigelb at redhat commented: There's a python3 compliance code now, I'm going to add in to sprint 5 to get an initial non-voting job in place for this.

gluster-ant commented 4 years ago

Time: 20181012T04:06:27 nigelb at redhat commented: Whoops, did not assign this to myself before changing status.