[Avocado-devel] Static code checking in avocado and avocado-vt

Fri Nov 20 01:22:45 UTC 2015

So, I have changed inspektor to not ignore the formerly ignored errors.
Let's comment some of those:

************* Module setup
I0011: 18,0: : Locally disabling no-name-in-module (E0611)
^ I0011 is trying to tell us that we shouldn't be adding python ignores to
the code. But guess what, we are ignoring
  the error for a reason. It's a false positive.

************* Module selftests.unit.test_vm
E1101: 54,8: VMTestResultTest.setUp: Class 'RemoteTestResult' has no
'should_receive' member
Pylint check fail: /home/lmr/Code/avocado/selftests/unit/test_vm.py
************* Module selftests.unit.test_utils_service
E1101:102,8: TestSpecificServiceManager.test_start: Instance of
'_SpecificServiceManager' has no 'start' member
E1101:108,8: TestSpecificServiceManager.test_stop_with_args: Instance of
'_SpecificServiceManager' has no 'stop' member
Pylint check fail:
/home/lmr/Code/avocado/selftests/unit/test_utils_service.py
************* Module selftests.unit.test_distro
E1101: 68,8: ProbeTest.test_name_for_file: Module 'posixpath' has no
'should_receive' member
E1101: 68,8: ProbeTest.test_name_for_file: Module 'ntpath' has no
'should_receive' member
E1101: 68,8: ProbeTest.test_name_for_file: Module 'os2emxpath' has no
'should_receive' member
Pylint check fail: /home/lmr/Code/avocado/selftests/unit/test_distro.py
************* Module selftests.unit.test_loader
I0011: 14,0: : Locally disabling protected-access (W0212)
************* Module selftests.functional.test_interrupt
E1101: 92,31: InterruptTest.test_badly_behaved.wait_until_no_badtest:
Module 'psutil' has no 'get_pid_list' member
E1101:145,31: InterruptTest.test_well_behaved.wait_until_no_goodtest:
Module 'psutil' has no 'get_pid_list' member
Pylint check fail:
/home/lmr/Code/avocado/selftests/functional/test_interrupt.py

^ Selftests mock stuff all around. We don't really want pylint telling us
that some members don't exist, we kinda know that already.

************* Module gdbtest
E1101:106,25: GdbTest.test_load_set_breakpoint_run_exit_raw: Instance of
'GdbMiRecord' has no 'class_' member
E1101:110,25: GdbTest.test_load_set_breakpoint_run_exit_raw: Instance of
'GdbMiRecord' has no 'class_' member
E1101:116,25: GdbTest.test_load_set_breakpoint_run_exit_raw: Instance of
'GdbMiRecord' has no 'class_' member
E1101:120,25: GdbTest.test_load_set_breakpoint_run_exit_raw: Instance of
'GdbMiRecord' has no 'class_' member
E1101:146,25: GdbTest.test_generate_core: Instance of 'GdbMiRecord' has no
'class_' member
E1101:150,25: GdbTest.test_generate_core: Instance of 'GdbMiRecord' has no
'class_' member
E1101:157,17: GdbTest.test_generate_core: Instance of 'GdbMiRecord' has no
'class_' member
E1101:163,33: GdbTest.test_generate_core: Instance of 'GdbMiRecord' has no
'class_' member
E1101:192,29: GdbTest.test_disconnect_raw: Instance of 'GdbMiRecord' has no
'class_' member
E1101:194,29: GdbTest.test_disconnect_raw: Instance of 'GdbMiRecord' has no
'class_' member
E1101:199,25: GdbTest.test_disconnect_raw: Instance of 'GdbMiRecord' has no
'class_' member
E1101:201,25: GdbTest.test_disconnect_raw: Instance of 'GdbMiRecord' has no
'class_' member
E1101:216,29: GdbTest.test_disconnect: Instance of 'GdbMiRecord' has no
'class_' member
E1101:218,29: GdbTest.test_disconnect: Instance of 'GdbMiRecord' has no
'class_' member
E1101:235,25: GdbTest.test_remote_exec: Instance of 'GdbMiRecord' has no
'class_' member
E1101:239,25: GdbTest.test_remote_exec: Instance of 'GdbMiRecord' has no
'class_' member
E1101:243,25: GdbTest.test_remote_exec: Instance of 'GdbMiRecord' has no
'class_' member
E1101:251,16: GdbTest.test_remote_exec: Instance of 'GdbMiRecord' has no
'class_' member
Pylint check fail: /home/lmr/Code/avocado/examples/tests/gdbtest.py
************* Module avocado.utils.gdb
E1101:454,16: GDB.cmd: Instance of 'GdbMiRecord' has no 'type' member
E1101:455,20: GDB.cmd: Instance of 'GdbMiRecord' has no 'record_type' member
E1101:457,17: GDB.cmd: Instance of 'GdbMiRecord' has no 'type' member
E1101:505,15: GDB.set_file: Instance of 'GdbMiRecord' has no 'class_' member
E1101:511,19: GDB.set_file: Instance of 'GdbMiRecord' has no 'class_' member
E1101:526,15: GDB.set_break: Instance of 'GdbMiRecord' has no 'class_'
member
E1101:542,15: GDB.del_break: Instance of 'GdbMiRecord' has no 'class_'
member
E1101:560,19: GDB.run: Instance of 'GdbMiRecord' has no 'class_' member
E1101:564,15: GDB.run: Instance of 'GdbMiRecord' has no 'class_' member
E1101:581,15: GDB.connect: Instance of 'GdbMiRecord' has no 'class_' member
E1101:595,15: GDB.disconnect: Instance of 'GdbMiRecord' has no 'class_'
member

^ Inspection of properties in C modules tends to be poor, and the examples
above show it.

Pylint check fail: /home/lmr/Code/avocado/avocado/utils/gdb.py
************* Module avocado.utils.process
I0011:772,0: : Locally disabling raising-bad-type (E0702)
I0011:781,0: : Locally disabling raising-bad-type (E0702)

^ OK pylint, we have been naughty boys and girls and are ignoring some
things, but guess what - we don't care.

E1101:658,15: GDBSubProcess.generate_core: Instance of 'GdbMiRecord' has no
'class_' member
Pylint check fail: /home/lmr/Code/avocado/avocado/utils/process.py
************* Module avocado.utils.data_structures
I0011:115,0: : Locally disabling bare-except (W0702)
************* Module avocado.utils.distro
I0011: 35,0: : Locally disabling too-few-public-methods (R0903)
************* Module avocado.utils.software_manager
E1101:171,19: BaseBackend.install_what_provides: Instance of 'BaseBackend'
has no 'provides' member
E1101:173,19: BaseBackend.install_what_provides: Instance of 'BaseBackend'
has no 'install' member

^ This has actually a point. I should change SoftwareManager to use more
standard coding techniques instead of what I did there. Oh well, I was too
young and didn't know better.

Pylint check fail: /home/lmr/Code/avocado/avocado/utils/software_manager.py
************* Module avocado.utils.service
I0011:425,0: : Locally disabling global-variable-undefined (W0601)
I0011:693,0: : Locally disabling global-variable-undefined (W0601)
I0011:713,0: : Locally disabling global-variable-undefined (W0601)
I0011:751,0: : Locally disabling global-variable-undefined (W0601)

^ ...

************* Module avocado.core.job
E1101:183,32: Job._make_test_runner: Instance of 'Namespace' has no
'test_runner' member
Pylint check fail: /home/lmr/Code/avocado/avocado/core/job.py
************* Module avocado.core.tree
I0011: 66,0: : Locally disabling too-few-public-methods (R0903)
I0011:307,0: : Locally disabling too-few-public-methods (R0903)
I0011:313,0: : Locally disabling too-few-public-methods (R0903)
I0011:369,0: : Locally disabling no-member (E1101)
I0011:536,0: : Locally disabling too-few-public-methods (R0903)
I0011:552,0: : Locally disabling too-few-public-methods (R0903)
I0011:577,0: : Locally disabling too-few-public-methods (R0903)
I0011:617,0: : Locally disabling too-few-public-methods (R0903)
I0011:644,0: : Locally disabling no-member (E1101)
I0011:654,0: : Locally disabling too-few-public-methods (R0903)
E1101:274,46: TreeNode.get_node: Instance of 'TreeNode' has no 'get_ascii'
member
Pylint check fail: /home/lmr/Code/avocado/avocado/core/tree.py
************* Module avocado.core.remoter
I0011:106,0: : Locally disabling raising-bad-type (E0702)
************* Module avocado.core.test
I0011:425,0: : Locally disabling raising-bad-type (E0702)
************* Module avocado.core.loader
I0011:281,0: : Locally disabling unused-argument (W0613)
************* Module avocado.core.plugins.htmlresult
I0011:237,0: : Locally disabling no-name-in-module (E0611)
************* Module avocado.core.remote.runner
E1101: 56,8: RemoteTestRunner.check_remote_avocado: Instance of 'MockClass'
has no 'remote' member
E1101: 56,8: RemoteTestRunner.check_remote_avocado: Instance of 'Mock' has
no 'remote' member
E1101: 58,17: RemoteTestRunner.check_remote_avocado: Instance of 'Mock' has
no 'remote' member
E1101: 58,17: RemoteTestRunner.check_remote_avocado: Instance of
'MockClass' has no 'remote' member
E1101: 79,45: RemoteTestRunner.run_test: Instance of 'MockClass' has no
'args' member
E1101: 79,45: RemoteTestRunner.run_test: Instance of 'Mock' has no 'args'
member
E1101: 84,19: RemoteTestRunner.run_test: Instance of 'Mock' has no 'args'
member
E1101: 84,19: RemoteTestRunner.run_test: Instance of 'MockClass' has no
'args' member
E1101: 90,28: RemoteTestRunner.run_test: Instance of 'Mock' has no 'remote'
member
E1101: 90,28: RemoteTestRunner.run_test: Instance of 'MockClass' has no
'remote' member
E1101: 98,44: RemoteTestRunner.run_test: Instance of 'Mock' has no 'stream'
member
E1101: 98,44: RemoteTestRunner.run_test: Instance of 'MockClass' has no
'stream' member
E1101:101,21: RemoteTestRunner.run_test: Instance of 'Mock' has no 'remote'
member
E1101:101,21: RemoteTestRunner.run_test: Instance of 'MockClass' has no
'remote' member
E1101:121,37: RemoteTestRunner.run_test: Instance of 'Mock' has no 'stream'
member
E1101:121,37: RemoteTestRunner.run_test: Instance of 'MockClass' has no
'stream' member
E1101:147,40: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'logdir' member
E1101:147,40: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'logdir' member
E1101:159,11: RemoteTestRunner.run_suite: Instance of 'Mock' has no 'args'
member
E1101:159,11: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'args' member
E1101:166,16: RemoteTestRunner.run_suite: Instance of 'Mock' has no 'setup'
member
E1101:166,16: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'setup' member
E1101:171,16: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'copy_files' member
E1101:171,16: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'copy_files' member
E1101:175,36: RemoteTestRunner.run_suite: Instance of 'Mock' has no 'urls'
member
E1101:175,36: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'urls' member
E1101:177,12: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'start_tests' member
E1101:177,12: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'start_tests' member
E1101:188,16: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'start_test' member
E1101:188,16: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'start_test' member
E1101:189,16: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'check_test' member
E1101:189,16: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'check_test' member
E1101:192,44: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'stream' member
E1101:192,44: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'stream' member
E1101:196,12: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'remote' member
E1101:196,12: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'remote' member
E1101:199,12: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'end_tests' member
E1101:199,12: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'end_tests' member
E1101:201,16: RemoteTestRunner.run_suite: Instance of 'Mock' has no
'tear_down' member
E1101:201,16: RemoteTestRunner.run_suite: Instance of 'MockClass' has no
'tear_down' member
Pylint check fail: /home/lmr/Code/avocado/avocado/core/remote/runner.py

^ Again, making member checking in unittests won't get you too far.

************* Module avocado.core.restclient.cli.parser
E1101:109,17: Parser.add_arguments_on_module: Instance of '_ArgumentGroup'
has no 'add_parser' member

^ Another false positive.

Pylint check fail:
/home/lmr/Code/avocado/avocado/core/restclient/cli/parser.py
Syntax check FAIL

So the bottom line is - static checking is great, but you have to select
what you're going to care about. Once again, if there are legitimate bugs
worth caring about, we can change our checkers to stop ignoring them.

I'm looking forward to see your pull requests.

Cheers,

Lucas

On Thu, Nov 19, 2015 at 10:59 PM Lucas Meneghel Rodrigues <lookkas at gmail.com>
wrote:

> On Thu, Nov 19, 2015 at 5:49 PM Alan Evangelista <
> alanoe at linux.vnet.ibm.com> wrote:
>
>> I have noticed avocado and avocado-vt don't pass pylint (1.4.3)
>> validation. Do you guys use any static code checker during development and
>> testing in Travis CI?
>>
>> If not, is there a reason why not? I use Pylint in all my Python projects
>> and it helps me a lot to detect bugs early, before running test suites. A
>> patch which makes code pass Pylint (or other static code checker of your
>> choice) validation would be viable and desired?
>>
>>
> I take personal offense by the thought that avocado and avocado-vt are not
> being regularly pylinted.
>
> Not only the CI always runs pylint checks, as they go through PEP8 and
> indentation checks. Example:
>
> https://travis-ci.org/avocado-framework/avocado/jobs/92175062
>
> Now, there are some errors that are ignored in inspektor:
>
> self.ignored_errors = 'E1002,E1101,E1103,E1120,F0401,I0011'
> Simply because they turned out to be false positives too frequently. If
> you want to show me that these ignored errors are actual bugs, by all
> means, send patches proving that, and then we'll start to fix the bugs
> across the board. We can make inspektor's config more flexible to select
> which errors are ignored for which project, then we can get the code to a
> better state.
>
> But even with other checkers, such as pydev on eclipse, and pycharm lint
> checker, I don't see any major issues in avocado.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20151120/9e5ef454/attachment.htm>