Refactoring: Redesign option parsing in push and merge scripts.

tools/push-to-trunk/test_scripts.py

Index: tools/push-to-trunk/test_scripts.py
diff --git a/tools/push-to-trunk/test_scripts.py b/tools/push-to-trunk/test_scripts.py
index 92aabf649296d1cfc3eb92942f6792818b2647ff..1693c108f2ea354bcca0649702a9f6a4f6fe1598 100644
--- a/tools/push-to-trunk/test_scripts.py
+++ b/tools/push-to-trunk/test_scripts.py
@@ -32,7 +32,6 @@ import traceback
 import unittest
 
 import auto_roll
-from auto_roll import AutoRollOptions
 from auto_roll import CheckLastPush
 from auto_roll import FetchLatestRevision
 from auto_roll import SETTINGS_LOCATION
@@ -64,24 +63,11 @@ TEST_CONFIG = {
   TEMPORARY_PATCH_FILE: "/tmp/test-merge-to-branch-tempfile-temporary-patch",
 }
 
-
-def MakeOptions(s=0, l=None, f=False, m=True, r=None, c=None, a=None,
-                status_password=None, revert_bleeding_edge=None, p=None):
-  """Convenience wrapper."""
-  class Options(object):
-      pass
-  options = Options()
-  options.s = s
-  options.l = l
-  options.f = f
-  options.m = m
-  options.reviewer = r
-  options.c = c
-  options.a = a
-  options.p = p
-  options.status_password = status_password
-  options.revert_bleeding_edge = revert_bleeding_edge
-  return options
+AUTO_ROLL_ARGS = [
+  "-a", "author@chromium.org",
+  "-c", TEST_CONFIG[CHROMIUM],
+  "-r", "reviewer@chromium.org",
+]
 
 
 class ToplevelTest(unittest.TestCase):
@@ -295,13 +281,17 @@ class ScriptTest(unittest.TestCase):
       f.write("#define IS_CANDIDATE_VERSION 0\n")
     return name
 
-  def MakeStep(self, step_class=Step, state=None, options=None):
+  def MakeStep(self):
+    """Convenience wrapper."""
+    options = ScriptsBase(TEST_CONFIG, self, self._state).MakeOptions([])
+    return MakeStep(step_class=Step, state=self._state,
+                    config=TEST_CONFIG, side_effect_handler=self,
+                    options=options)
+
+  def RunStep(self, script=PushToTrunk, step_class=Step, args=None):
     """Convenience wrapper."""
-    options = options or CommonOptions(MakeOptions())
-    state = state if state is not None else self._state
-    return MakeStep(step_class=step_class, number=0, state=state,
-                    config=TEST_CONFIG, options=options,
-                    side_effect_handler=self)
+    args = args or ["-m"]
+    return script(TEST_CONFIG, self, self._state).RunSteps([step_class], args)
 
   def GitMock(self, cmd, args="", pipe=True):
     print "%s %s" % (cmd, args)
@@ -483,7 +473,7 @@ class ScriptTest(unittest.TestCase):
     ])
 
     self._state["last_push_bleeding_edge"] = "1234"
-    self.MakeStep(PrepareChangeLog).Run()
+    self.RunStep(PushToTrunk, PrepareChangeLog)
 
     actual_cl = FileToText(TEST_CONFIG[CHANGELOG_ENTRY_FILE])
 
@@ -530,8 +520,7 @@ class ScriptTest(unittest.TestCase):
       "",  # Open editor.
     ])
 
-    self.MakeStep(EditChangeLog).Run()
-
+    self.RunStep(PushToTrunk, EditChangeLog)
     self.assertEquals("New\n        Lines\n\n\n        Original CL",
                       FileToText(TEST_CONFIG[CHANGELOG_FILE]))
 
@@ -543,8 +532,7 @@ class ScriptTest(unittest.TestCase):
       "Y",  # Increment build number.
     ])
 
-    self.MakeStep(IncrementVersion).Run()
-
+    self.RunStep(PushToTrunk, IncrementVersion)
     self.assertEquals("3", self._state["new_major"])
     self.assertEquals("22", self._state["new_minor"])
     self.assertEquals("6", self._state["new_build"])
@@ -579,7 +567,7 @@ class ScriptTest(unittest.TestCase):
     self._state["prepare_commit_hash"] = "hash1"
     self._state["date"] = "1999-11-11"
 
-    self.MakeStep(SquashCommits).Run()
+    self.RunStep(PushToTrunk, SquashCommits)
     self.assertEquals(FileToText(TEST_CONFIG[COMMITMSG_FILE]), expected_msg)
 
     patch = FileToText(TEST_CONFIG[ PATCH_FILE])
@@ -740,10 +728,11 @@ Performance and stability improvements on all platforms.""", commit)
     if force:
       self.ExpectReadline([])
 
-    options = MakeOptions(f=force, m=manual, a="author@chromium.org",
-                          r="reviewer@chromium.org" if not manual else None,
-                          c = TEST_CONFIG[CHROMIUM])
-    RunPushToTrunk(TEST_CONFIG, PushToTrunkOptions(options), self)
+    args = ["-a", "author@chromium.org", "-c", TEST_CONFIG[CHROMIUM]]
+    if force: args.append("-f")
+    if manual: args.append("-m")
+    else: args += ["-r", "reviewer@chromium.org"]
+    PushToTrunk(TEST_CONFIG, self).Run(args)
 
     deps = FileToText(TEST_CONFIG[DEPS_FILE])
     self.assertTrue(re.search("\"v8_revision\": \"123456\"", deps))
@@ -772,13 +761,14 @@ Performance and stability improvements on all platforms.""", commit)
       ["svn log -1 --oneline ChangeLog", "r99 | Prepare push to trunk..."],
     ])
 
-    state = {}
-    self.MakeStep(FetchLatestRevision, state=state).Run()
-    self.assertRaises(Exception, self.MakeStep(CheckLastPush, state=state).Run)
+    self.RunStep(auto_roll.AutoRoll, FetchLatestRevision, AUTO_ROLL_ARGS)
+    self.assertRaises(Exception, lambda: self.RunStep(auto_roll.AutoRoll,
+                                                      CheckLastPush,
+                                                      AUTO_ROLL_ARGS))
 
   def testAutoRoll(self):
-    status_password = self.MakeEmptyTempFile()
-    TextToFile("PW", status_password)
+    password = self.MakeEmptyTempFile()
+    TextToFile("PW", password)
     TEST_CONFIG[DOT_GIT_LOCATION] = self.MakeEmptyTempFile()
     TEST_CONFIG[SETTINGS_LOCATION] = "~/.doesnotexist"
 
@@ -807,8 +797,8 @@ Performance and stability improvements on all platforms.""", commit)
       ["svn find-rev push_hash", "65"],
     ])
 
-    auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(
-        MakeOptions(status_password=status_password)), self)
+    auto_roll.AutoRoll(TEST_CONFIG, self).Run(
+        AUTO_ROLL_ARGS + ["--status-password", password])
 
     state = json.loads(FileToText("%s-state.json"
                                   % TEST_CONFIG[PERSISTFILE_BASENAME]))
@@ -830,7 +820,7 @@ Performance and stability improvements on all platforms.""", commit)
     ])
 
     def RunAutoRoll():
-      auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(MakeOptions()), self)
+      auto_roll.AutoRoll(TEST_CONFIG, self).Run(AUTO_ROLL_ARGS)
     self.assertRaises(Exception, RunAutoRoll)
 
   def testAutoRollStoppedByTreeStatus(self):
@@ -849,7 +839,7 @@ Performance and stability improvements on all platforms.""", commit)
     ])
 
     def RunAutoRoll():
-      auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(MakeOptions()), self)
+      auto_roll.AutoRoll(TEST_CONFIG, self).Run(AUTO_ROLL_ARGS)
     self.assertRaises(Exception, RunAutoRoll)
 
   def testMergeToBranch(self):
@@ -966,27 +956,23 @@ LOG=N
       "LGTM",  # Enter LGTM for V8 CL.
     ])
 
-    options = MakeOptions(p=extra_patch, f=True)
     # r12345 and r34567 are patches. r23456 (included) and r45678 are the MIPS
     # ports of r12345. r56789 is the MIPS port of r34567.
-    args = ["trunk", "12345", "23456", "34567"]
-    self.assertTrue(merge_to_branch.ProcessOptions(options, args))
+    args = ["-f", "-p", extra_patch, "--branch", "trunk", "12345", "23456",
+            "34567"]
 
     # The first run of the script stops because of the svn being down.
     self.assertRaises(GitFailedException,
-        lambda: RunMergeToBranch(TEST_CONFIG,
-                                 MergeToBranchOptions(options, args),
-                                 self))
+        lambda: MergeToBranch(TEST_CONFIG, self).Run(args))
 
     # Test that state recovery after restarting the script works.
-    options.s = 3
-    RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options, args), self)
+    args += ["-s", "3"]
+    MergeToBranch(TEST_CONFIG, self).Run(args)
 
 
 class SystemTest(unittest.TestCase):
   def testReload(self):
     step = MakeStep(step_class=PrepareChangeLog, number=0, state={}, config={},
-                    options=CommonOptions(MakeOptions()),
                     side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER)
     body = step.Reload(
 """------------------------------------------------------------------------