Skip to content

Commit

Permalink
GitCommitBear.py: Detect revert commits
Browse files Browse the repository at this point in the history
This detects revert commits
and checks they have a reason as well.

Closes #337
  • Loading branch information
kriti21 committed May 6, 2018
1 parent 3773249 commit f98d234
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 4 deletions.
55 changes: 53 additions & 2 deletions bears/vcs/git/GitCommitBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ def check_prerequisites(cls):
else:
return True

@classmethod
def get_revert_checks_metadata(cls):
return FunctionMetadata.from_function(
cls.check_revert,
omit={'self', 'shortlog', 'body'})

@classmethod
def get_shortlog_checks_metadata(cls):
return FunctionMetadata.from_function(
Expand All @@ -91,6 +97,7 @@ def get_metadata(cls):
FunctionMetadata.from_function(
cls.run,
omit={'self', 'dependency_results'}),
cls.get_revert_checks_metadata(),
cls.get_shortlog_checks_metadata(),
cls.get_body_checks_metadata(),
cls.get_issue_checks_metadata())
Expand All @@ -112,8 +119,7 @@ def get_host_from_remotes():
netloc = parsed_url.host
return netloc.split('.')[0]

def run(self,
allow_empty_commit_message: bool = False,
def run(self, allow_empty_commit_message: bool = False,
**kwargs):
"""
Check the current git commit message at HEAD.
Expand Down Expand Up @@ -141,6 +147,10 @@ def run(self,
yield Result(self, 'HEAD commit has no message.')
return

yield from self.check_revert(
shortlog,
body,
**self.get_revert_checks_metadata().filter_parameters(kwargs))
yield from self.check_shortlog(
shortlog,
**self.get_shortlog_checks_metadata().filter_parameters(kwargs))
Expand All @@ -151,6 +161,47 @@ def run(self,
body,
**self.get_issue_checks_metadata().filter_parameters(kwargs))

def check_revert(self, shortlog, body,
allow_revert_commits_without_reason: bool = False,):
"""
Validates revert commits.
:param shortlog: The shortlog message string.
:param body: The body of the commit message of HEAD.
:param allow_revert_commits_without_reason: Whether reason is
needed for revert commits.
"""
REVERT_REGEX = re.compile(r'(Revert ")([\S+\s*]*)("$)')
HEADLINE_REGEX = re.compile(r'(^This reverts commit )([0-9a-f]{40})')

if REVERT_REGEX.match(shortlog):
reverted_shortlog = re.search(REVERT_REGEX, shortlog).group(2)

body = body.strip('\n')
blankline = body.find('\n\n')
headline = body[:blankline] if blankline != -1 else body

if not HEADLINE_REGEX.match(headline):
yield Result(self, 'Invalid revert commit.')
return

commit_sha = re.search(HEADLINE_REGEX, headline).group(2)
command = ('git log -n 1 ' +
str(commit_sha) + ' --pretty=%B')
rstdout, _ = run_shell_command(command)
rstdout = rstdout.rstrip('\n')
rpos = rstdout.find('\n')
old_shortlog = rstdout[:rpos] if rpos != -1 else rstdout

if old_shortlog != reverted_shortlog:
yield Result(self, 'Shortlog of revert commit does '
'not match the original commit.')

reason = body[blankline+1:] if blankline != -1 else ''

if not allow_revert_commits_without_reason and len(reason) == 0:
yield Result(self, 'Revert commit does not have a reason.')

def check_shortlog(self, shortlog,
shortlog_length: int = 50,
shortlog_regex: str = '',
Expand Down
60 changes: 58 additions & 2 deletions tests/vcs/git/GitCommitBearTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,13 @@ def test_get_metadata(self):
metadata = GitCommitBear.get_metadata()
self.assertEqual(
metadata.name,
"<Merged signature of 'run', 'check_shortlog', 'check_body'"
", 'check_issue_reference'>")
"<Merged signature of 'run', 'check_revert', 'check_shortlog'"
", 'check_body', 'check_issue_reference'>")

# Test if at least one parameter of each signature is used.
self.assertIn('allow_empty_commit_message', metadata.optional_params)
self.assertIn('allow_revert_commits_without_reason',
metadata.optional_params)
self.assertIn('shortlog_length', metadata.optional_params)
self.assertIn('body_line_length', metadata.optional_params)
self.assertIn('body_close_issue', metadata.optional_params)
Expand All @@ -125,6 +127,60 @@ def test_empty_message(self):
[])
self.assert_no_msgs()

def test_revert_checks(self):
self.git_commit('Test commit')
commit_hash, _ = run_shell_command('git rev-parse HEAD')
self.git_commit('Revert "Test commit"\n\n'
'Abcdef blablabla commit ' + str(commit_hash) + '\n\n'
'Reason for the revert commit is explained here.\n')
self.assertEqual(self.run_uut(),
['Invalid revert commit.'])
self.assert_no_msgs()

self.git_commit('Test commit1\n\n'
'This is body of test commit1\n')
commit_hash, _ = run_shell_command('git rev-parse HEAD')

self.git_commit('Revert "Another Test commit2"\n\n'
'This reverts commit ' + str(commit_hash) + '\n\n'
'Reason for the revert commit is explained here.\n')
self.assertEqual(
self.run_uut(),
['Shortlog of revert commit does not match the original commit.'])
self.assert_no_msgs()

self.git_commit('Test commit3')
commit_hash, _ = run_shell_command('git rev-parse HEAD')
self.git_commit('Revert "Test commit3"\n\n'
'This reverts commit ' + str(commit_hash) + '\n\n')
self.assertEqual(self.run_uut(),
['Revert commit does not have a reason.'])
self.assert_no_msgs()

self.git_commit('Revert "Test commit3"\n\n'
'This reverts commit ' + str(commit_hash) + '\n\n')
self.assertEqual(
self.run_uut(allow_revert_commits_without_reason=True),
[])
self.assert_no_msgs()

self.git_commit('Revert "Test commit3"\n\n'
'This reverts commit ' + str(commit_hash) + '\n\n'
'Reason for the revert commit is explained here.\n')
self.assertEqual(self.run_uut(), [])
self.assert_no_msgs()

self.git_commit('Test commit4\n\n'
'This is body of test commit1\n')
commit_hash, _ = run_shell_command('git rev-parse HEAD')
self.git_commit('Revert "Another Test commit5"\n\n'
'This reverts commit ' + str(commit_hash) + '\n\n'
'Reason for the revert commit is explained here.\n')
self.assertEqual(
self.run_uut(),
['Shortlog of revert commit does not match the original commit.'])
self.assert_no_msgs()

@unittest.mock.patch('bears.vcs.git.GitCommitBear.run_shell_command',
return_value=('one-liner-message\n', ''))
def test_pure_oneliner_message(self, patch):
Expand Down

0 comments on commit f98d234

Please sign in to comment.