Make auto-roll testable.

Refactor the mock code for easier reuse. Mock out web requests.

TEST=python -m unittest test_scripts
BUG=
R=ulan@chromium.org

Review URL: https://codereview.chromium.org/77453009

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17987 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

tools/push-to-trunk/auto_roll.py

@@ -29,7 +29,6 @@
 import optparse
 import re
 import sys
-import urllib2
 
 from common_includes import *
 
@@ -66,15 +65,7 @@ class FetchLKGR(Step):
 
   def RunStep(self):
     lkgr_url = "https://v8-status.appspot.com/lkgr"
-    try:
+    self.Persist("lkgr", self.ReadURL(lkgr_url))
-      # pylint: disable=E1121
-      url_fh = urllib2.urlopen(lkgr_url, None, 60)
-    except urllib2.URLError:
-      self.Die("URLException while fetching %s" % lkgr_url)
-    try:
-      self.Persist("lkgr", url_fh.read())
-    finally:
-      url_fh.close()
 
 
 class PushToTrunk(Step):
@@ -94,6 +85,18 @@ class PushToTrunk(Step):
             % (latest, lkgr))
 
 
+def RunAutoRoll(config,
+                options,
+                side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
+  step_classes = [
+    Preparation,
+    FetchLatestRevision,
+    FetchLKGR,
+    PushToTrunk,
+  ]
+  RunScript(step_classes, config, options, side_effect_handler)
+
+
 def BuildOptions():
   result = optparse.OptionParser()
   result.add_option("-s", "--step", dest="s",
@@ -105,15 +108,7 @@ def BuildOptions():
 def Main():
   parser = BuildOptions()
   (options, args) = parser.parse_args()
-
+  RunAutoRoll(CONFIG, options)
-  step_classes = [
-    Preparation,
-    FetchLatestRevision,
-    FetchLKGR,
-    PushToTrunk,
-  ]
-
-  RunScript(step_classes, CONFIG, options, DEFAULT_SIDE_EFFECT_HANDLER)
 
 if __name__ == "__main__":
   sys.exit(Main())

tools/push-to-trunk/common_includes.py

@@ -31,6 +31,7 @@ import re
 import subprocess
 import sys
 import textwrap
+import urllib2
 
 PERSISTFILE_BASENAME = "PERSISTFILE_BASENAME"
 TEMP_BRANCH = "TEMP_BRANCH"
@@ -192,6 +193,14 @@ class SideEffectHandler(object):
   def ReadLine(self):
     return sys.stdin.readline().strip()
 
+  def ReadURL(self, url):
+    # pylint: disable=E1121
+    url_fh = urllib2.urlopen(url, None, 60)
+    try:
+      return url_fh.read()
+    finally:
+      url_fh.close()
+
 DEFAULT_SIDE_EFFECT_HANDLER = SideEffectHandler()
 
 
@@ -251,6 +260,9 @@ class Step(object):
     return self._side_effect_handler.Command(os.environ["EDITOR"], args,
                                              pipe=False)
 
+  def ReadURL(self, url):
+    return self._side_effect_handler.ReadURL(url)
+
   def Die(self, msg=""):
     if msg != "":
       print "Error: %s" % msg

tools/push-to-trunk/test_scripts.py

@@ -34,6 +34,7 @@ import common_includes
 from common_includes import *
 import push_to_trunk
 from push_to_trunk import *
+import auto_roll
 
 
 TEST_CONFIG = {
@@ -178,6 +179,51 @@ class ToplevelTest(unittest.TestCase):
                                                 "BUG=1234567890123456789\n"
                                                 "BUG=1234567890\n"))
 
+
+class SimpleMock(object):
+  def __init__(self, name):
+    self._name = name
+    self._recipe = []
+    self._index = -1
+
+  def Expect(self, recipe):
+    self._recipe = recipe
+
+  def Call(self, *args):
+    self._index += 1
+    try:
+      expected_call = self._recipe[self._index]
+    except IndexError:
+      raise Exception("Calling %s %s" % (name, " ".join(args)))
+
+    # Pack expectations without arguments into a list.
+    if not isinstance(expected_call, list):
+      expected_call = [expected_call]
+
+    # The number of arguments in the expectation must match the actual
+    # arguments.
+    if len(args) > len(expected_call):
+      raise Exception("When calling %s with arguments, the expectations "
+                      "must consist of at least as many arguments.")
+
+    # Compare expected and actual arguments.
+    for (expected_arg, actual_arg) in zip(expected_call, args):
+      if expected_arg != actual_arg:
+        raise Exception("Expected: %s - Actual: %s"
+                        % (expected_arg, actual_arg))
+
+    # The expectation list contains a mandatory return value and an optional
+    # callback for checking the context at the time of the call.
+    if len(expected_call) == len(args) + 2:
+      expected_call[len(args) + 1]()
+    return expected_call[len(args)]
+
+  def AssertFinished(self):
+    if self._index < len(self._recipe) -1:
+      raise Exception("Called %s too seldom: %d vs. %d"
+                      % (self._name, self._index, len(self._recipe)))
+
+
 class ScriptTest(unittest.TestCase):
   def MakeEmptyTempFile(self):
     handle, name = tempfile.mkstemp()
@@ -208,17 +254,7 @@ class ScriptTest(unittest.TestCase):
     return step
 
   def GitMock(self, cmd, args="", pipe=True):
-    self._git_index += 1
+    return self._git_mock.Call(args)
-    try:
-      git_invocation = self._git_recipe[self._git_index]
-    except IndexError:
-      raise Exception("Calling git %s" % args)
-    if git_invocation[0] != args:
-      raise Exception("Expected: %s - Actual: %s" % (git_invocation[0], args))
-    if len(git_invocation) == 3:
-      # Run optional function checking the context during this git command.
-      git_invocation[2]()
-    return git_invocation[1]
 
   def LogMock(self, cmd, args=""):
     print "Log: %s %s" % (cmd, args)
@@ -232,17 +268,27 @@ class ScriptTest(unittest.TestCase):
     return ScriptTest.MOCKS[cmd](self, cmd, args)
 
   def ReadLine(self):
-    self._rl_index += 1
+    return self._rl_mock.Call()
-    try:
+
-      return self._rl_recipe[self._rl_index]
+  def ReadURL(self, url):
-    except IndexError:
+    return self._url_mock.Call(url)
-      raise Exception("Calling readline too often")
+
+  def ExpectGit(self, *args):
+    """Convenience wrapper."""
+    self._git_mock.Expect(*args)
+
+  def ExpectReadline(self, *args):
+    """Convenience wrapper."""
+    self._rl_mock.Expect(*args)
+
+  def ExpectReadURL(self, *args):
+    """Convenience wrapper."""
+    self._url_mock.Expect(*args)
 
   def setUp(self):
-    self._git_recipe = []
+    self._git_mock = SimpleMock("git")
-    self._git_index = -1
+    self._rl_mock = SimpleMock("readline")
-    self._rl_recipe = []
+    self._url_mock = SimpleMock("readurl")
-    self._rl_index = -1
     self._tmp_files = []
 
   def tearDown(self):
@@ -253,12 +299,9 @@ class ScriptTest(unittest.TestCase):
       if os.path.exists(name):
         os.remove(name)
 
-    if self._git_index < len(self._git_recipe) -1:
+    self._git_mock.AssertFinished()
-      raise Exception("Called git too seldom: %d vs. %d" %
+    self._rl_mock.AssertFinished()
-                      (self._git_index, len(self._git_recipe)))
+    self._url_mock.AssertFinished()
-    if self._rl_index < len(self._rl_recipe) -1:
-      raise Exception("Too little input: %d vs. %d" %
-                      (self._rl_index, len(self._rl_recipe)))
 
   def testPersistRestore(self):
     self.MakeStep().Persist("test1", "")
@@ -270,12 +313,12 @@ class ScriptTest(unittest.TestCase):
     self.assertTrue(Command("git", "--version").startswith("git version"))
 
   def testGitMock(self):
-    self._git_recipe = [["--version", "git version 1.2.3"], ["dummy", ""]]
+    self.ExpectGit([["--version", "git version 1.2.3"], ["dummy", ""]])
     self.assertEquals("git version 1.2.3", self.MakeStep().Git("--version"))
     self.assertEquals("", self.MakeStep().Git("dummy"))
 
   def testCommonPrepareDefault(self):
-    self._git_recipe = [
+    self.ExpectGit([
       ["status -s -uno", ""],
       ["status -s -b -uno", "## some_branch"],
       ["svn fetch", ""],
@@ -283,33 +326,33 @@ class ScriptTest(unittest.TestCase):
       ["branch -D %s" % TEST_CONFIG[TEMP_BRANCH], ""],
       ["checkout -b %s" % TEST_CONFIG[TEMP_BRANCH], ""],
       ["branch", ""],
-    ]
+    ])
-    self._rl_recipe = ["Y"]
+    self.ExpectReadline(["Y"])
     self.MakeStep().CommonPrepare()
     self.MakeStep().PrepareBranch()
     self.assertEquals("some_branch", self.MakeStep().Restore("current_branch"))
 
   def testCommonPrepareNoConfirm(self):
-    self._git_recipe = [
+    self.ExpectGit([
       ["status -s -uno", ""],
       ["status -s -b -uno", "## some_branch"],
       ["svn fetch", ""],
       ["branch", "  branch1\n* %s" % TEST_CONFIG[TEMP_BRANCH]],
-    ]
+    ])
-    self._rl_recipe = ["n"]
+    self.ExpectReadline(["n"])
     self.MakeStep().CommonPrepare()
     self.assertRaises(Exception, self.MakeStep().PrepareBranch)
     self.assertEquals("some_branch", self.MakeStep().Restore("current_branch"))
 
   def testCommonPrepareDeleteBranchFailure(self):
-    self._git_recipe = [
+    self.ExpectGit([
       ["status -s -uno", ""],
       ["status -s -b -uno", "## some_branch"],
       ["svn fetch", ""],
       ["branch", "  branch1\n* %s" % TEST_CONFIG[TEMP_BRANCH]],
       ["branch -D %s" % TEST_CONFIG[TEMP_BRANCH], None],
-    ]
+    ])
-    self._rl_recipe = ["Y"]
+    self.ExpectReadline(["Y"])
     self.MakeStep().CommonPrepare()
     self.assertRaises(Exception, self.MakeStep().PrepareBranch)
     self.assertEquals("some_branch", self.MakeStep().Restore("current_branch"))
@@ -357,7 +400,7 @@ class ScriptTest(unittest.TestCase):
     TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile()
     TEST_CONFIG[CHANGELOG_ENTRY_FILE] = self.MakeEmptyTempFile()
 
-    self._git_recipe = [
+    self.ExpectGit([
       ["log 1234..HEAD --format=%H", "rev1\nrev2\nrev3"],
       ["log -1 rev1 --format=\"%w(80,8,8)%s\"", "        Title text 1"],
       ["log -1 rev1 --format=\"%B\"", "Title\n\nBUG=\nLOG=y\n"],
@@ -371,7 +414,7 @@ class ScriptTest(unittest.TestCase):
       ["log -1 rev3 --format=\"%B\"", "Title\n\nBUG=321\nLOG=true\n"],
       ["log -1 rev3 --format=\"%w(80,8,8)(%an)\"",
        "        author3@chromium.org"],
-    ]
+    ])
 
     self.MakeStep().Persist("last_push", "1234")
     self.MakeStep(PrepareChangeLog).Run()
@@ -419,9 +462,9 @@ class ScriptTest(unittest.TestCase):
     TextToFile("  New  \n\tLines  \n", TEST_CONFIG[CHANGELOG_ENTRY_FILE])
     os.environ["EDITOR"] = "vi"
 
-    self._rl_recipe = [
+    self.ExpectReadline([
       "",  # Open editor.
-    ]
+    ])
 
     self.MakeStep(EditChangeLog).Run()
 
@@ -432,9 +475,9 @@ class ScriptTest(unittest.TestCase):
     TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile()
     self.MakeStep().Persist("build", "5")
 
-    self._rl_recipe = [
+    self.ExpectReadline([
       "Y",  # Increment build number.
-    ]
+    ])
 
     self.MakeStep(IncrementVersion).Run()
 
@@ -470,9 +513,9 @@ class ScriptTest(unittest.TestCase):
       f.write("        Performance and stability improvements on all "
               "platforms.\n")
 
-    self._git_recipe = [
+    self.ExpectGit([
       ["diff svn/trunk hash1", "patch content"],
-    ]
+    ])
 
     self.MakeStep().Persist("prepare_commit_hash", "hash1")
     self.MakeStep().Persist("date", "1999-11-11")
@@ -528,7 +571,7 @@ class ScriptTest(unittest.TestCase):
       self.assertTrue(re.search(r"#define IS_CANDIDATE_VERSION\s+0", version))
 
     force_flag = " -f" if force else ""
-    self._git_recipe = [
+    self.ExpectGit([
       ["status -s -uno", ""],
       ["status -s -b -uno", "## some_branch\n"],
       ["svn fetch", ""],
@@ -575,8 +618,8 @@ class ScriptTest(unittest.TestCase):
       ["branch -D %s" % TEST_CONFIG[TEMP_BRANCH], ""],
       ["branch -D %s" % TEST_CONFIG[BRANCHNAME], ""],
       ["branch -D %s" % TEST_CONFIG[TRUNKBRANCH], ""],
-    ]
+    ])
-    self._rl_recipe = [
+    self.ExpectReadline([
       "Y",  # Confirm last push.
       "",  # Open editor.
       "Y",  # Increment build number.
@@ -585,13 +628,13 @@ class ScriptTest(unittest.TestCase):
       "LGTM",  # Enter LGTM for V8 CL.
       "Y",  # Sanity check.
       "reviewer@chromium.org",  # Chromium reviewer.
-    ]
+    ])
     if force:
       # TODO(machenbach): The lgtm for the prepare push is just temporary.
       # There should be no user input in "force" mode.
-      self._rl_recipe = [
+      self.ExpectReadline([
         "LGTM",  # Enter LGTM for V8 CL.
-      ]
+      ])
 
     class Options( object ):
       pass
@@ -622,3 +665,32 @@ class ScriptTest(unittest.TestCase):
 
   def testPushToTrunkForced(self):
     self._PushToTrunk(force=True)
+
+  def testAutoRoll(self):
+    TEST_CONFIG[DOT_GIT_LOCATION] = self.MakeEmptyTempFile()
+
+    # TODO(machenbach): Get rid of the editor check in automatic mode.
+    os.environ["EDITOR"] = "vi"
+
+    self.ExpectReadURL([
+      ["https://v8-status.appspot.com/lkgr", "100"],
+    ])
+
+    self.ExpectGit([
+      ["status -s -uno", ""],
+      ["status -s -b -uno", "## some_branch\n"],
+      ["svn fetch", ""],
+      ["svn log -1 --oneline", "r101 | Text"],
+    ])
+
+    # TODO(machenbach): Make a convenience wrapper for this.
+    class Options( object ):
+      pass
+
+    options = Options()
+    options.s = 0
+
+    auto_roll.RunAutoRoll(TEST_CONFIG, options, self)
+
+    self.assertEquals("100", self.MakeStep().Restore("lkgr"))
+    self.assertEquals("101", self.MakeStep().Restore("latest"))