search

Capitains / HookTest

Testing script for Hook
Mozilla Public License 2.0
3 stars 3 forks source link

Warning cleanup #143

Closed rillian closed 5 years ago

rillian commented 5 years ago

Resolve the majority of errors reported by pycodestyle/pytest/setup.py. Tested under python 3.7.

Please consider merging these. I was trying to figure out why HookTest wasn't reporting anything on my test corpus and then started looking at the code, but I wanted to be able to see any new errors from changes I made debugging. Reducing the warning output makes that easier.

This PR reduces pycodestyle --ignore E501 *.py HookTest from 48 lines to 8.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.004%) to 95.71% when pulling 3f068140c7cdfcd49e273430c9c76f82373f8c23 on rillian:warnings into 2090d9195556bd692343db54e1702ba2b839e9f4 on Capitains:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.008%) to 95.698% when pulling bbabc23f28df6471233c65f6b700bdb8da32d9ca on rillian:warnings into 2090d9195556bd692343db54e1702ba2b839e9f4 on Capitains:master.

PonteIneptique commented 5 years ago

BTW, if this continues to not report anything, feel free to share the corpus in an issue and I'll gladly see if HookTest is missing something or is bugged :)

rillian commented 5 years ago

Thanks for the quick review!

As long as you add these clean-ups, can you automate these checkings in the .travis.yml ? Thanks in advance ;)

I've gone ahead and fixed the remaining style warnings (except for long lines) and added pycodestyle to the travis run. It will new report failures for any new warnings that are introduced.

rillian commented 5 years ago

I think the coverage changes might be rounding error? I can't find a line where the coverage is different.

PonteIneptique commented 5 years ago

I am evaluating this as I write. Sorry for this huge delay.

PonteIneptique commented 5 years ago

Thanks for this Pull Request and sorry for the delay ;)

rillian commented 5 years ago

Thanks for reviewing. It was an extensive set of changes. To me it sometimes seems like Python strings are as confusing as Rust strings!

PonteIneptique commented 5 years ago

What is the most confusing to me with this introduction is how this will affect stuff such as f.read() containing such characters. Obviously, it will (or maybe not) not throw errors, but then, two strings have different behavior...

rillian commented 5 years ago

I think the invalid escape sequence warnings (soon to be errors) only apply to string literals. That is, strings created from text in surrounded by quotes in a Python file. That's where the backslash-escape system is applied to create strings with characters which aren't easy to represent.

Strings created through f.read() come off the disk already containing whatever such characters they do, and escape sequences aren't interpreted. They can still raise an exception and fail to create a string if they're not valid in the expected character encoding, though.