[Release] Distinguish between merges and follow-up CLs

tools/release/test_mergeinfo.py

+#!/usr/bin/env python
+# Copyright 2015 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import mergeinfo
+import shutil
+import unittest
+
+from collections import namedtuple
+from os import path
+from subprocess import Popen, PIPE, check_call
+
+TEST_CONFIG = {
+  "GIT_REPO": "/tmp/test-v8-search-related-commits",
+}
+
+class TestMergeInfo(unittest.TestCase):
+
+  base_dir = TEST_CONFIG["GIT_REPO"]
+
+  def _execute_git(self, git_args):
+
+    fullCommand = ["git", "-C", self.base_dir] + git_args
+    p = Popen(args=fullCommand, stdin=PIPE,
+        stdout=PIPE, stderr=PIPE)
+    output, err = p.communicate()
+    rc = p.returncode
+    if rc != 0:
+      raise Exception(err)
+    return output
+
+  def setUp(self):
+    if path.exists(self.base_dir):
+      shutil.rmtree(self.base_dir)
+
+    check_call(["git", "init", self.base_dir])
+
+    # Initial commit
+    message = '''Initial commit'''
+
+    self._make_empty_commit(message)
+
+  def tearDown(self):
+    if path.exists(self.base_dir):
+      shutil.rmtree(self.base_dir)
+
+  def _assert_correct_standard_result(
+      self, result, all_commits, hash_of_first_commit):
+    self.assertEqual(len(result), 1, "Master commit not found")
+    self.assertTrue(
+        result.get(hash_of_first_commit),
+        "Master commit is wrong")
+
+    self.assertEqual(
+        len(result[hash_of_first_commit]),
+        1,
+        "Child commit not found")
+    self.assertEqual(
+        all_commits[2],
+        result[hash_of_first_commit][0],
+        "Child commit wrong")
+
+  def _get_commits(self):
+    commits = self._execute_git(
+        ["log", "--format=%H", "--reverse"]).splitlines()
+    return commits
+
+  def _make_empty_commit(self, message):
+    self._execute_git(["commit", "--allow-empty", "-m", message])
+
+  def testCanDescribeCommit(self):
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]
+
+    result = mergeinfo.describe_commit(
+        self.base_dir,
+        hash_of_first_commit).splitlines()
+
+    self.assertEqual(
+        result[0],
+        'commit ' + hash_of_first_commit)
+
+  def testCanDescribeCommitSingleLine(self):
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]
+
+    result = mergeinfo.describe_commit(
+        self.base_dir,
+        hash_of_first_commit, True).splitlines()
+
+    self.assertEqual(
+        str(result[0]),
+        str(hash_of_first_commit[0:7]) + ' Initial commit')
+
+  def testSearchFollowUpCommits(self):
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]
+
+    message = 'Follow-up commit of '  + hash_of_first_commit
+    self._make_empty_commit(message)
+    self._make_empty_commit(message)
+    self._make_empty_commit(message)
+    commits = self._get_commits()
+    message = 'Not related commit'
+    self._make_empty_commit(message)
+
+    followups = mergeinfo.get_followup_commits(
+        self.base_dir,
+        hash_of_first_commit)
+    self.assertEqual(set(followups), set(commits[1:]))
+
+  def testSearchMerges(self):
+    self._execute_git(['branch', 'test'])
+    self._execute_git(['checkout', 'master'])
+    message = 'real initial commit'
+    self._make_empty_commit(message)
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]
+
+    self._execute_git(['checkout', 'test'])
+    message = 'Not related commit'
+    self._make_empty_commit(message)
+
+    #This should be found

[Michael Achenbach, 2015/10/01 08:24:18]

nit: space after #
also everywhere below

[Michael Hablich, 2015/10/01 10:33:25]

Done.

+    message = 'Merge '  + hash_of_first_commit
+    self._make_empty_commit(message)
+    hash_of_hit = self._get_commits()[-1]
+
+    #This should be ignored
+    message = 'Cr-Branched-From: '  + hash_of_first_commit
+    self._make_empty_commit(message)

[Michael Achenbach, 2015/10/01 08:24:18]

Suggestion: You could let _make_empty_commit return the hash of what just was
created. In most of the cases you need an extra line for fetching it.

[Michael Hablich, 2015/10/01 10:33:26]

I like it.

+    hash_of_ignored = self._get_commits()[-1]
+
+    self._execute_git(['checkout', 'master'])
+
+    followups = mergeinfo.get_followup_commits(
+        self.base_dir,
+        hash_of_first_commit)
+
+    #Check if follow ups and merges are not overlapping
+    self.assertEqual(len(followups), 0)

[Michael Achenbach, 2015/10/01 08:24:18]

I assume if you'd checked out a branch where a merge is on by any chance, you'd
find the merge as a follow up.

[Michael Hablich, 2015/10/01 10:33:26]

Do you mean what will happen if you are applying this script not on master but
on a release branch? It shouldn't matter because 'master' is hardwired.

[Michael Achenbach, 2015/10/01 10:40:29]

Right!

+
+    message = 'Follow-up commit of '  + hash_of_first_commit
+    self._make_empty_commit(message)
+    hash_of_followup = self._get_commits()[-1]
+
+    merges = mergeinfo.get_merge_commits(self.base_dir, hash_of_first_commit)
+    #Check if follow up is ignored
+    self.assertTrue(hash_of_followup not in merges)
+
+    #Check for proper return of merges
+    self.assertTrue(hash_of_hit in merges)
+    self.assertTrue(hash_of_ignored not in merges)
+
+  def testIsRolled(self):
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]

[Michael Achenbach, 2015/10/01 08:24:18]

The test (also for lkgr) would be more covering if roll (or lkgr) wouldn't point
to the exact commit, but to some commit on top.

[Michael Hablich, 2015/10/01 10:33:26]

Ack.

Also added another assert.

+    self._execute_git(['branch', 'remotes/origin/roll'])

[Michael Achenbach, 2015/10/01 08:24:18]

Remember that the roll ref is just storing what should be rolled - not what is
rolled. We don't have the latter (we only have the canary ref).

[Michael Hablich, 2015/10/01 10:33:26]

You are right. I changed the wording.

+
+    self.assertTrue(mergeinfo.is_rolled(
+      self.base_dir, hash_of_first_commit))
+
+  def testIsLkgr(self):
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]
+    self._execute_git(['branch', 'remotes/origin/lkgr'])
+
+    self.assertTrue(mergeinfo.is_lkgr(
+      self.base_dir, hash_of_first_commit))
+
+  def testShowFirstCanary(self):
+    commits = self._get_commits()
+    hash_of_first_commit = commits[0]
+
+    self.assertEqual(mergeinfo.get_first_canary(
+      self.base_dir, hash_of_first_commit), 'No Canary coverage')
+
+    self._execute_git(['branch', 'remotes/origin/chromium/2345'])
+    self._execute_git(['branch', 'remotes/origin/chromium/2346'])
+
+    self.assertEqual(mergeinfo.get_first_canary(
+      self.base_dir, hash_of_first_commit), '2345')
+
+if __name__ == "__main__":
+  #import sys;sys.argv = ['', 'Test.testName']

[Michael Achenbach, 2015/10/01 08:24:18]

nit: remove?

[Michael Hablich, 2015/10/01 10:33:25]

Done.

+   unittest.main()