CheckstyleBear: Detect errors #1527
Conversation
| # Force DOS format, as Checkstyle configs enable NewlineAtEndOfFile, | ||
| # which defaults to CRLF on Windows, and Appveyor CI ignores .gitattributes | ||
| # http://help.appveyor.com/discussions/problems/5687-gitattributes-changes-dont-have-any-effect | ||
| - unix2dos tests/java/test_files/CheckstyleGood.java |
There was a problem hiding this comment.
let's do this with the bad file as well so we see the differences between the configs.
There was a problem hiding this comment.
I dont object to doing that; certainly makes sense.
However pushing back a little bit, and only a little bit, this line fixes one problem occurring in test_style_android, and it occurs only on Windows. The bad files already fail, for other reasons, and fail on unix, so the only thing which more unix2dos would find is more Windows specific issues wrt checkstyle.
When I upgraded to Checkstyle 7.6 (#1466), there are three failed method - test_style_sun and test_style_google also fail on Appveyor and not on Travis/Circle , so there are more Windows specific problems somewhere in there, which will be found in the next set of patches for CheckstyleBear.
jayvdb
force-pushed
the
checkstyle-ERROR
branch
from
7c1eb9b to
bb4c2ff
Compare
gitmate-bot
added
process/pending review
review carefully!
and removed
process/wip
labels
bears/java/CheckstyleBear.py
Outdated
| @@ -33,7 +33,7 @@ def known_checkstyle_or_path(setting): | |||
|
|
|||
| @linter(executable='java', | |||
| output_format='regex', | |||
| output_regex=r'\[(?P<severity>WARN|INFO)\].*?' | |||
| output_regex=r'\[(?P<severity>WARN|INFO|ERROR)\].*?' | |||
There was a problem hiding this comment.
Small thing:
Why not order it info warn error?
There was a problem hiding this comment.
Good catch. It seems (git grep '<severity>') ERROR|WARN|INFO would be the more common ordering.
jayvdb
force-pushed
the
checkstyle-ERROR
branch
2 times, most recently
from
f63088a to
0d4d895
Compare
|
|
||
| def test_style_sun(self): | ||
| self.section['checkstyle_configs'] = 'sun' | ||
| self.check_validity(self.uut, [], self.good_file) | ||
| self.check_validity(self.uut, [], self.good_file, valid=False) |
There was a problem hiding this comment.
valid=False? Is that non-Sun-compliant?
There was a problem hiding this comment.
yup. I'm surprised it is compliant with any ruleset to be honest.
|
ack 0d4d895 |
|
needs rebase |
The first commit coala/coala@9bea8fe80, and the rewrite in 9227382, ignored ERROR from checkstyle. Also set java files to be native EOL in .gitattributes, to match default EOL expected by Checkstyle rule NewlineAtEndOfFile, and force DOS EOL on AppVeyor. Also remove `test_run` as it uses `google` by default, which is covered in the specifically named test method. Fixes coala#1464
jayvdb
force-pushed
the
checkstyle-ERROR
branch
from
0d4d895 to
d9234ef
Compare
|
reack d9234ef |
|
@rultor merge |
Since 9227382, errors from checkstyle were ignored.
Also set java files to be native EOL in .gitattributes,
to match default EOL expected by Checkstyle rule
NewlineAtEndOfFile.
Fixes #1464