Checkstyle handling when all lines are deleted.

It is valid to have empty modified_lines list for files
that only contain deletions. In such case we should skip
all the errors except for forced rules.

Updated the test to match the expectation.

Change-Id: I6993968b882fb6fbe2ba1f63f3b6879c3308ff34
This commit is contained in:
Aurimas Liutikas
2016-01-21 14:43:12 -08:00
parent b571663f91
commit a0e889762d
2 changed files with 6 additions and 2 deletions

View File

@@ -130,7 +130,10 @@ def _ParseAndFilterOutput(stdout, sha, last_commit_modified_files):
# Returns whether an error on a given line should be skipped # Returns whether an error on a given line should be skipped
# based on the modified_lines list and the rule. # based on the modified_lines list and the rule.
def _ShouldSkip(modified_lines, line, rule): def _ShouldSkip(modified_lines, line, rule):
return modified_lines and line not in modified_lines and rule not in FORCED_RULES # None modified_lines means checked file is new and nothing should be skipped.
if modified_lines is None:
return False
return line not in modified_lines and rule not in FORCED_RULES
def _GetModifiedFiles(out = sys.stdout): def _GetModifiedFiles(out = sys.stdout):

View File

@@ -77,7 +77,8 @@ class TestCheckstyle(unittest.TestCase):
checkstyle.git.last_commit = mock_last_commit checkstyle.git.last_commit = mock_last_commit
def test_ShouldSkip(self): def test_ShouldSkip(self):
self.assertFalse(checkstyle._ShouldSkip([], 1, TEST_RULE)) self.assertFalse(checkstyle._ShouldSkip(None, 1, TEST_RULE))
self.assertTrue(checkstyle._ShouldSkip([], 1, TEST_RULE))
self.assertFalse(checkstyle._ShouldSkip([1], 1, TEST_RULE)) self.assertFalse(checkstyle._ShouldSkip([1], 1, TEST_RULE))
self.assertFalse(checkstyle._ShouldSkip([1, 2, 3], 1, TEST_RULE)) self.assertFalse(checkstyle._ShouldSkip([1, 2, 3], 1, TEST_RULE))
self.assertTrue(checkstyle._ShouldSkip([1, 2, 3], 4, TEST_RULE)) self.assertTrue(checkstyle._ShouldSkip([1, 2, 3], 4, TEST_RULE))