From 8986022b6421de15c78d346199f1e773f6a8d07d Mon Sep 17 00:00:00 2001
From: Ernesto Puerta <epuertat@redhat.com>
Date: Thu, 13 Sep 2018 15:12:09 +0200
Subject: [PATCH] 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>
---
 ceph-pr-commits/build/test_commits.py | 89 ++++++++++-----------------
 1 file changed, 34 insertions(+), 55 deletions(-)

diff --git a/ceph-pr-commits/build/test_commits.py b/ceph-pr-commits/build/test_commits.py
index 0debe990..4dc4c829 100644
--- a/ceph-pr-commits/build/test_commits.py
+++ b/ceph-pr-commits/build/test_commits.py
@@ -1,68 +1,47 @@
-from subprocess import Popen, PIPE
+from subprocess import check_output
 import os
 from os.path import dirname
 import pytest
+import shlex
 
-# 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')
-
-
-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()
-
-
-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')
 
+    @staticmethod
+    def command(command, cwd=None):
+        print "Running command: %s" % (command,)
+        return check_output(shlex.split(command), cwd=cwd)
 
-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),
+            cwd=cls.ceph_checkout)
 
     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 = 'Signed-off-by: \S.* <[^@]+@[^@]+\.[^@]+>'
+        check_signed_off_commits = 'git log -E --invert-grep --grep="%s" --no-merges origin/%s..%s' % (
+                signed_off_regex,
+                self.target_branch,
+                self.source_branch)
+        wrong_commits = self.command(check_signed_off_commits)
+        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
-- 
2.39.5