ceph-pr-commits: Fix check with many-commit PRs

Relevant changes:
- All non-compliant commits are reported (previously, only the first
    wrong one was reported, stopping further processing).
- Signed-off-by regex put in place to check for
    <something@somewhere.tld> (including lt and gt symbols). This rule
    could be relaxed, as some commits in the past were sent without
    the enclosing <...>
- `extract_sha` function has been removed

Fixes: http://tracker.ceph.com/issues/36082
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>

diff --git a/ceph-pr-commits/build/test_commits.py b/ceph-pr-commits/build/test_commits.py
index 0debe9904572b61f5c1e26f7c44040a9eb038c57..d48be08cddbd44a4ece721e4a12c06be86392eae 100644 (file)
--- a/ceph-pr-commits/build/test_commits.py
+++ b/ceph-pr-commits/build/test_commits.py
@@ -1,68 +1,50 @@
-from subprocess import Popen, PIPE
+from subprocess import check_output
 import os
 from os.path import dirname
-import pytest
-
-# ceph-pr-commits/build
-current_directory = dirname(os.path.abspath(__file__))
-
-# workspace directory
-workspace = os.getenv('WORKSPACE', None) or dirname(dirname(dirname(current_directory)))
-
-# ceph checkout path
-ceph_checkout = os.path.join(workspace, 'ceph')
+import shlex
+import re
+from itertools import ifilterfalse
 
-
-def run(command):
-    print "Running command: %s" % ' '.join(command)
-    process = Popen(
-        command,
-        cwd=ceph_checkout,
-        stdout=PIPE,
-        stderr=PIPE,
-        close_fds=True
-    )
-
-    returncode = process.wait()
-
-    return process.stdout.read()
+import pytest
 
 
-def get_commits():
+class TestCommits(object):
+    """
+    This class will contain all checks required for commits
+    """
     target_branch = os.getenv('ghprbTargetBranch', 'master')
-    # ensure that we have the latest commits from master
-    command = ['git', 'fetch', 'origin', '+refs/heads/{target_branch}:refs/remotes/origin/{target_branch}'.format(target_branch=target_branch)]
-    run(command)
-    # we use 'HEAD' here because the PR is already checked out on the right branch
     source_branch = 'HEAD'
-    command = ['git', 'log', '--no-merges', 'origin/%s..%s' % (target_branch, source_branch)]
-    output = run(command)
-    chunked_commits = []
-    for chunk in output.split('\n\ncommit'):
-        if not chunk:
-            continue
-        chunked_commits.append(chunk)
-    return chunked_commits
 
+    workspace = os.getenv('WORKSPACE') or dirname(dirname(dirname(dirname(os.path.abspath(__file__)))))
+    ceph_checkout = os.path.join(workspace, 'ceph')
 
+    @classmethod
+    def command(cls, command):
+        print "Running command: %s" % (command,)
+        return check_output(shlex.split(command), cwd=cls.ceph_checkout)
 
-class TestSignedOffByCommits(object):
+    @classmethod
+    def setup_class(cls):
+        # ensure that we have the latest commits from master
+        cls.command(
+            'git fetch origin +refs/heads/{target_branch}:refs/remotes/origin/{target_branch}'.format(
+                target_branch=cls.target_branch))
 
     def test_signed_off_by(self):
-        for commit in get_commits():
-            if 'Signed-off-by:' not in commit:
-                msg = (
-                    "\nFollowing commit is not signed, please make sure all commits",
-                    "\nare signed following the 'Submitting Patches' guide:",
-                    "\nhttps://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work",
-                    "\n",
-                    commit)
-                raise AssertionError, ' '.join(msg)
+        signed_off_regex = r'Signed-off-by: \S.* <[^@]+@[^@]+\.[^@]+>'
+        # '-z' puts a '\0' between commits, see later split('\0')
+        check_signed_off_commits = 'git log -z --no-merges origin/%s..%s' % (
+                self.target_branch, self.source_branch)
+        wrong_commits = list(ifilterfalse(
+                re.compile(signed_off_regex).search,
+                self.command(check_signed_off_commits).split('\0')))
+        if wrong_commits:
+            raise AssertionError("\n".join([
+                "Following commit/s is/are not signed, please make sure all TestCommits",
+                "are signed following the 'Submitting Patches' guide:",
+                "https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work",
+                ""] + 
+                wrong_commits
+                )
+            )
 
-    def extract_sha(self, lines):
-        # XXX Unused for now, if py.test can spit out the hashes in verbose
-        # mode this should be removed, otherwise put to good use
-        trim = lines.split()
-        for i in trim:
-            if i and 'commit' not in i:
-                return i