avocado-framework / inspektor

Inspektor code checker
Other
11 stars 17 forks source link

Inspekt fails with `--fix` parameter #27

Closed rdkdd closed 7 years ago

rdkdd commented 7 years ago

Without fix option inspektor run in Travis does not fail, but if I run inspekt checkall --exclude=.git --fix in Travis, it silently fails. autopep8 was installed.

lmr commented 7 years ago

Ok, are you running it with --fix because you want to see the changes necessary before fixing them or what? It does not strike me as a very useful flag for CI.

lmr commented 7 years ago

I just found out what your problem probably is, the license checker is probably finding a file that it couldn't properly detect as having a license.

The license check functionality is more brittle than I expected. I'm going to exclude that from checkall and explain it in the help text.

rdkdd commented 7 years ago

I think it does not depend on Travis environment. If I type inspekt checkall, lets say it PASSes. Then with inspekt chackall --fix it FAILs (!) (I can not find reason why in neither of verbose/debug/log output), but if I run inspekt chackall --fix for the second time, then it PASSes:

inspekt checkall:

[2017-08-09 14:26:20,026] DEBUG    inspekt initialize_app
[2017-08-09 14:26:20,111] DEBUG    inspekt prepare_to_run_command CheckAllCommand
[2017-08-09 14:26:20,112] INFO     inspektor.commands.checkall PEP8 disabled: E501,E265,W601,E402
[2017-08-09 14:26:20,112] INFO     inspektor.commands.checkall Pylint disabled: W,R,C,E1002,E1101,E1103,E1120,F0401,I0011
[2017-08-09 14:26:20,112] INFO     inspektor.commands.checkall Pylint enabled : W0611
[2017-08-09 14:26:20,113] DEBUG    root Lint: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:21,462] DEBUG    root Lint: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:22,002] DEBUG    root Indent: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:22,006] DEBUG    root Indent: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:22,010] DEBUG    root Style: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:22,032] DEBUG    root Style: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:22,064] DEBUG    root License: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:22,064] DEBUG    root License: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:22,064] INFO     inspektor.commands.checkall Global check PASS
[2017-08-09 14:26:22,064] DEBUG    inspekt clean_up CheckAllCommand

inspekt checkall --fix:

[2017-08-09 14:26:30,550] DEBUG    inspekt initialize_app
[2017-08-09 14:26:30,633] DEBUG    inspekt prepare_to_run_command CheckAllCommand
[2017-08-09 14:26:30,635] INFO     inspektor.commands.checkall PEP8 disabled: E501,E265,W601,E402
[2017-08-09 14:26:30,635] INFO     inspektor.commands.checkall Pylint disabled: W,R,C,E1002,E1101,E1103,E1120,F0401,I0011
[2017-08-09 14:26:30,635] INFO     inspektor.commands.checkall Pylint enabled : W0611
[2017-08-09 14:26:30,636] DEBUG    root Lint: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:31,959] DEBUG    root Lint: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:32,484] DEBUG    root Indent: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:32,487] DEBUG    root Indent: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:32,492] DEBUG    root Style: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:32,513] DEBUG    root Style: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:32,545] DEBUG    root License: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:32,545] DEBUG    root License: /home/rduda/git/tp-spice/spice/deps/build_install.py
[2017-08-09 14:26:32,546] ERROR    inspektor.commands.checkall Global check FAIL
[2017-08-09 14:26:32,546] DEBUG    inspekt clean_up CheckAllCommand

output of second inspekt checkall --fixis the same as inspekt checkall

I would appreciate some warning message, when the command is run with --fix flag and autopep8 is not installed.

lmr commented 7 years ago

@rdkdd, sure, although the current code in master is supposed to log an error message when you don't have autopep8 installed. I'm looking into your issue and see what a good solution might be.

lmr commented 7 years ago

@rdkdd I'd recommend git reset, then inspekt chackall --fix, then git diff. This will show you what inspektor has fixed. Since it has fixed the issue, the second run PASSes, so this particular thing you mentioned is expected behavior.

lmr commented 7 years ago

I'd guess the problem happened around here:

[2017-08-09 14:26:32,545] DEBUG    root License: /home/rduda/git/tp-spice/spice/deps/helper_cb.py
[2017-08-09 14:26:32,545] DEBUG    root License: /home/rduda/git/tp-spice/spice/deps/build_install.py

The License checker is too brittle, and doesn't report properly, I'm going to fix that. I'm also removing license checking from 'checkall', since it was mostly tought for a very narrow use case (projects that will use GPL v2 or GPL v2+). I'm sorry for the trouble, let me try to fix this ASAP.

lmr commented 7 years ago

Yep, as I suspected, the message about the lack of autopep8 is there:

[2017-08-09 15:29:51,757] ERROR    inspektor.commands.checkall Style check fail: /home/lmr/Code/Avocado/inspektor/inspektor/style.py
[2017-08-09 15:29:51,757] ERROR    inspektor.commands.checkall Python library autopep8 not installed. Please install it if you want to use --fix
lmr commented 7 years ago

Please check https://github.com/avocado-framework/inspektor/pull/28/commits/9d66cc5c13fbb99c70e7bb927b68d1f217ecb94c, it should solve your issue. I'll merge shortly and release if someone can help me with the review.

rdkdd commented 7 years ago

@lmr I checked this and the commit solves the problem. Thanks!