Refactoring: Make script dependencies more object-oriented in push and merge scripts.

- Connect the top-level scripts via inheritance and remove top-level functions
- Options and option processing can be extended from base to subclass script

This CL is split off from https://codereview.chromium.org/173983002/

BUG=
R=ulan@chromium.org

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

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

tools/push-to-trunk/auto_roll.py

@@ -35,8 +35,6 @@ import urllib
 
 from common_includes import *
 import push_to_trunk
-from push_to_trunk import PushToTrunkOptions
-from push_to_trunk import RunPushToTrunk
 
 SETTINGS_LOCATION = "SETTINGS_LOCATION"
 
@@ -47,16 +45,6 @@ CONFIG = {
 }
 
 
-class AutoRollOptions(CommonOptions):
-  def __init__(self, options):
-    super(AutoRollOptions, self).__init__(options)
-    self.requires_editor = False
-    self.status_password = options.status_password
-    self.chromium = options.chromium
-    self.push = getattr(options, 'push', False)
-    self.author = getattr(options, 'author', None)
-
-
 class Preparation(Step):
   MESSAGE = "Preparation."
 
@@ -151,12 +139,11 @@ class PushToTrunk(Step):
       try:
         if self._options.push:
           self._side_effect_handler.Call(
-              RunPushToTrunk,
+              PushToTrunk(push_to_trunk.CONFIG, self._side_effect_handler).Run,
-              push_to_trunk.CONFIG,
+              ["-a", self._options.author,
-              PushToTrunkOptions.MakeForcedOptions(self._options.author,
+               "-c", self._options.chromium,
-                                                   self._options.reviewer,
+               "-r", self._options.reviewer,
-                                                   self._options.chromium),
+               "-f"])
-              self._side_effect_handler)
       finally:
         self.PushTreeStatus(self["tree_message"])
     else:
@@ -164,49 +151,35 @@ class PushToTrunk(Step):
             % (latest, lkgr))
 
 
-def RunAutoRoll(config,
+class AutoRoll(ScriptsBase):
-                options,
+  def _PrepareOptions(self, parser):
-                side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
+    parser.add_argument("-c", "--chromium", required=True,
-  step_classes = [
+                        help=("The path to your Chromium src/ "
-    Preparation,
+                              "directory to automate the V8 roll."))
-    CheckAutoRollSettings,
+    parser.add_argument("-p", "--push",
-    CheckTreeStatus,
+                        help="Push to trunk. Dry run if unspecified.",
-    FetchLatestRevision,
+                        default=False, action="store_true")
-    CheckLastPush,
+    parser.add_argument("--status-password",
-    FetchLKGR,
+                        help="A file with the password to the status app.")
-    PushToTrunk,
-  ]
-  RunScript(step_classes, config, options, side_effect_handler)
 
+  def _ProcessOptions(self, options):
+    if not options.author or not options.reviewer:
+      print "You need to specify author and reviewer."
+      return False
+    options.requires_editor = False
+    return True
 
-def BuildOptions():
+  def _Steps(self):
-  parser = argparse.ArgumentParser()
+    return [
-  parser.add_argument("-a", "--author",
+      Preparation,
-                      help="The author email used for rietveld.")
+      CheckAutoRollSettings,
-  parser.add_argument("-c", "--chromium",
+      CheckTreeStatus,
-                      help=("The path to your Chromium src/ directory to "
+      FetchLatestRevision,
-                            "automate the V8 roll."))
+      CheckLastPush,
-  parser.add_argument("-p", "--push",
+      FetchLKGR,
-                      help="Push to trunk if possible. Dry run if unspecified.",
+      PushToTrunk,
-                      default=False, action="store_true")
+    ]
-  parser.add_argument("-r", "--reviewer",
-                      help="The account name to be used for reviews.")
-  parser.add_argument("-s", "--step",
-                      help="Specify the step where to start work. Default: 0.",
-                      default=0, type=int)
-  parser.add_argument("--status-password",
-                      help="A file with the password to the status app.")
-  return parser
 
 
-def Main():
-  parser = BuildOptions()
-  options = parser.parse_args()
-  if not options.author or not options.chromium or not options.reviewer:
-    print "You need to specify author, chromium src location and reviewer."
-    parser.print_help()
-    return 1
-  RunAutoRoll(CONFIG, AutoRollOptions(options))
-
 if __name__ == "__main__":
-  sys.exit(Main())
+  sys.exit(AutoRoll(CONFIG).Run())

tools/push-to-trunk/common_includes.py

@@ -26,6 +26,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import argparse
 import datetime
 import json
 import os
@@ -226,18 +227,6 @@ class GitFailedException(Exception):
   pass
 
 
-class CommonOptions(object):
-  def __init__(self, options, manual=True):
-    self.requires_editor = True
-    self.wait_for_lgtm = True
-    self.step = options.step
-    self.force_readline_defaults = not manual
-    self.force_upload = not manual
-    self.manual = manual
-    self.reviewer = getattr(options, 'reviewer', "")
-    self.author = getattr(options, 'author', "")
-
-
 class Step(GitRecipesMixin):
   def __init__(self, text, requires, number, config, state, options, handler):
     self._text = text
@@ -251,7 +240,6 @@ class Step(GitRecipesMixin):
     assert self._config is not None
     assert self._state is not None
     assert self._side_effect_handler is not None
-    assert isinstance(options, CommonOptions)
 
   def __getitem__(self, key):
     # Convenience method to allow direct [] access on step classes for
@@ -502,18 +490,81 @@ def MakeStep(step_class=Step, number=0, state=None, config=None,
                       handler=side_effect_handler)
 
 
-def RunScript(step_classes,
+class ScriptsBase(object):
-              config,
+  # TODO(machenbach): Move static config here.
-              options,
+  def __init__(self, config, side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER,
-              side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
+               state=None):
-  state_file = "%s-state.json" % config[PERSISTFILE_BASENAME]
+    self._config = config
-  if options.step == 0 and os.path.exists(state_file):
+    self._side_effect_handler = side_effect_handler
-    os.remove(state_file)
+    self._state = state if state is not None else {}
-  state = {}
-  steps = []
-  for (number, step_class) in enumerate(step_classes):
-    steps.append(MakeStep(step_class, number, state, config,
-                          options, side_effect_handler))
 
-  for step in steps[options.step:]:
+  def _Description(self):
-    step.Run()
+    return None
+
+  def _PrepareOptions(self, parser):
+    pass
+
+  def _ProcessOptions(self, options):
+    return True
+
+  def _Steps(self):
+    raise Exception("Not implemented.")
+
+  def MakeOptions(self, args=None):
+    parser = argparse.ArgumentParser(description=self._Description())
+    parser.add_argument("-a", "--author", default="",
+                        help="The author email used for rietveld.")
+    parser.add_argument("-r", "--reviewer", default="",
+                        help="The account name to be used for reviews.")
+    parser.add_argument("-s", "--step",
+                      help="Specify the step where to start work. Default: 0.",
+                      default=0, type=int)
+
+    self._PrepareOptions(parser)
+
+    if args is None:
+      options = parser.parse_args()
+    else:
+      options = parser.parse_args(args)
+
+    # Process common options.
+    if options.step < 0:
+      print "Bad step number %d" % options.step
+      parser.print_help()
+      return None
+
+    # Defaults for options, common to all scripts.
+    options.manual = getattr(options, "manual", True)
+    options.force = getattr(options, "force", False)
+
+    # Derived options.
+    options.requires_editor = not options.force
+    options.wait_for_lgtm = not options.force
+    options.force_readline_defaults = not options.manual
+    options.force_upload = not options.manual
+
+    # Process script specific options.
+    if not self._ProcessOptions(options):
+      parser.print_help()
+      return None
+    return options
+
+  def RunSteps(self, step_classes, args=None):
+    options = self.MakeOptions(args)
+    if not options:
+      return 1
+
+    state_file = "%s-state.json" % self._config[PERSISTFILE_BASENAME]
+    if options.step == 0 and os.path.exists(state_file):
+      os.remove(state_file)
+
+    steps = []
+    for (number, step_class) in enumerate(step_classes):
+      steps.append(MakeStep(step_class, number, self._state, self._config,
+                            options, self._side_effect_handler))
+    for step in steps[options.step:]:
+      step.Run()
+    return 0
+
+  def Run(self, args=None):
+    return self.RunSteps(self._Steps(), args)

tools/push-to-trunk/merge_to_branch.py

@@ -50,26 +50,12 @@ CONFIG = {
 }
 
 
-class MergeToBranchOptions(CommonOptions):
-  def __init__(self, options):
-    super(MergeToBranchOptions, self).__init__(options, True)
-    self.requires_editor = True
-    self.wait_for_lgtm = True
-    self.delete_sentinel = options.f
-    self.message = getattr(options, "message", "")
-    self.revert = getattr(options, "r", False)
-    self.revert_bleeding_edge = getattr(options, "revert_bleeding_edge", False)
-    self.patch = getattr(options, "p", "")
-    self.branch = options.branch
-    self.revisions = options.revisions
-
-
 class Preparation(Step):
   MESSAGE = "Preparation."
 
   def RunStep(self):
     if os.path.exists(self.Config(ALREADY_MERGING_SENTINEL_FILE)):
-      if self._options.delete_sentinel:
+      if self._options.force:
         os.remove(self.Config(ALREADY_MERGING_SENTINEL_FILE))
       elif self._options.step == 0:
         self.Die("A merge is already in progress")
@@ -288,76 +274,58 @@ class CleanUp(Step):
         print "patches: %s" % self["revision_list"]
 
 
-def RunMergeToBranch(config,
+class MergeToBranch(ScriptsBase):
-                     options,
+  def _Description(self):
-                     side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
+    return ("Performs the necessary steps to merge revisions from "
-  step_classes = [
+            "bleeding_edge to other branches, including trunk.")
-    Preparation,
-    CreateBranch,
-    SearchArchitecturePorts,
-    FindGitRevisions,
-    ApplyPatches,
-    PrepareVersion,
-    IncrementVersion,
-    CommitLocal,
-    UploadStep,
-    CommitRepository,
-    PrepareSVN,
-    TagRevision,
-    CleanUp,
-  ]
 
-  RunScript(step_classes, config, options, side_effect_handler)
+  def _PrepareOptions(self, parser):
+    group = parser.add_mutually_exclusive_group(required=True)
+    group.add_argument("--branch", help="The branch to merge to.")
+    group.add_argument("-R", "--revert-bleeding-edge",
+                       help="Revert specified patches from bleeding edge.",
+                       default=False, action="store_true")
+    parser.add_argument("revisions", nargs="*",
+                        help="The revisions to merge.")
+    parser.add_argument("-f", "--force",
+                        help="Delete sentinel file.",
+                        default=False, action="store_true")
+    parser.add_argument("-m", "--message",
+                        help="A commit message for the patch.")
+    parser.add_argument("--revert",
+                        help="Revert specified patches.",
+                        default=False, action="store_true")
+    parser.add_argument("-p", "--patch",
+                        help="A patch file to apply as part of the merge.")
 
+  def _ProcessOptions(self, options):
+    # TODO(machenbach): Add a test that covers revert from bleeding_edge
+    if len(options.revisions) < 1:
+      if not options.patch:
+        print "Either a patch file or revision numbers must be specified"
+        return False
+      if not options.message:
+        print "You must specify a merge comment if no patches are specified"
+        return False
+    return True
 
-def BuildOptions():
+  def _Steps(self):
-  parser = argparse.ArgumentParser(
+    return [
-      description=("Performs the necessary steps to merge revisions from "
+      Preparation,
-                   "bleeding_edge to other branches, including trunk."))
+      CreateBranch,
-  group = parser.add_mutually_exclusive_group(required=True)
+      SearchArchitecturePorts,
-  group.add_argument("--branch", help="The branch to merge to.")
+      FindGitRevisions,
-  group.add_argument("-R", "--revert-bleeding-edge",
+      ApplyPatches,
-                     help="Revert specified patches from bleeding edge.",
+      PrepareVersion,
-                     default=False, action="store_true")
+      IncrementVersion,
-  parser.add_argument("revisions", nargs="*",
+      CommitLocal,
-                      help="The revisions to merge.")
+      UploadStep,
-  parser.add_argument("-a", "--author", default="",
+      CommitRepository,
-                       help="The author email used for rietveld.")
+      PrepareSVN,
-  parser.add_argument("-f",
+      TagRevision,
-                      help="Delete sentinel file.",
+      CleanUp,
-                      default=False, action="store_true")
+    ]
-  parser.add_argument("-m", "--message",
-                      help="A commit message for the patch.")
-  parser.add_argument("-r", "--revert",
-                      help="Revert specified patches.",
-                      default=False, action="store_true")
-  parser.add_argument("-p", "--patch", dest="p",
-                      help="A patch file to apply as part of the merge.")
-  parser.add_argument("-s", "--step",
-                      help="The step where to start work. Default: 0.",
-                      default=0, type=int)
-  return parser
 
 
-def ProcessOptions(options):
-  # TODO(machenbach): Add a test that covers revert from bleeding_edge
-  if len(options.revisions) < 1:
-    if not options.patch:
-      print "Either a patch file or revision numbers must be specified"
-      return False
-    if not options.message:
-      print "You must specify a merge comment if no patches are specified"
-      return False
-  return True
-
-
-def Main():
-  parser = BuildOptions()
-  options = parser.parse_args()
-  if not ProcessOptions(options):
-    parser.print_help()
-    return 1
-  RunMergeToBranch(CONFIG, MergeToBranchOptions(options))
-
 if __name__ == "__main__":
-  sys.exit(Main())
+  sys.exit(MergeToBranch(CONFIG).Run())

tools/push-to-trunk/push_to_trunk.py

@@ -55,35 +55,6 @@ PUSH_MESSAGE_SUFFIX = " (based on bleeding_edge revision r%d)"
 PUSH_MESSAGE_RE = re.compile(r".* \(based on bleeding_edge revision r(\d+)\)$")
 
 
-class PushToTrunkOptions(CommonOptions):
-  @staticmethod
-  def MakeForcedOptions(author, reviewer, chrome_path):
-    """Convenience wrapper."""
-    class Options(object):
-      pass
-    options = Options()
-    options.step = 0
-    options.last_push = None
-    options.last_bleeding_edge = None
-    options.force = True
-    options.manual = False
-    options.chromium = chrome_path
-    options.reviewer = reviewer
-    options.author = author
-    return PushToTrunkOptions(options)
-
-  def __init__(self, options):
-    super(PushToTrunkOptions, self).__init__(options, options.manual)
-    self.requires_editor = not options.force
-    self.wait_for_lgtm = not options.force
-    self.tbr_commit = not options.manual
-    self.last_push = options.last_push
-    self.reviewer = options.reviewer
-    self.chromium = options.chromium
-    self.last_bleeding_edge = getattr(options, 'last_bleeding_edge', None)
-    self.author = getattr(options, 'author', None)
-
-
 class Preparation(Step):
   MESSAGE = "Preparation."
 
@@ -511,90 +482,67 @@ class CleanUp(Step):
       self.GitDeleteBranch(self.Config(TRUNKBRANCH))
 
 
-def RunPushToTrunk(config,
+class PushToTrunk(ScriptsBase):
-                   options,
+  def _PrepareOptions(self, parser):
-                   side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
+    group = parser.add_mutually_exclusive_group()
-  step_classes = [
+    group.add_argument("-f", "--force",
-    Preparation,
+                      help="Don't prompt the user.",
-    FreshBranch,
+                      default=False, action="store_true")
-    DetectLastPush,
+    group.add_argument("-m", "--manual",
-    PrepareChangeLog,
+                      help="Prompt the user at every important step.",
-    EditChangeLog,
+                      default=False, action="store_true")
-    IncrementVersion,
+    parser.add_argument("-b", "--last-bleeding-edge",
-    CommitLocal,
+                        help=("The git commit ID of the last bleeding edge "
-    UploadStep,
+                              "revision that was pushed to trunk. This is "
-    CommitRepository,
+                              "used for the auto-generated ChangeLog entry."))
-    StragglerCommits,
+    parser.add_argument("-c", "--chromium",
-    SquashCommits,
+                        help=("The path to your Chromium src/ "
-    NewBranch,
+                              "directory to automate the V8 roll."))
-    ApplyChanges,
+    parser.add_argument("-l", "--last-push",
-    SetVersion,
+                        help="The git commit ID of the last push to trunk.")
-    CommitTrunk,
-    SanityCheck,
-    CommitSVN,
-    TagRevision,
-    CheckChromium,
-    SwitchChromium,
-    UpdateChromiumCheckout,
-    UploadCL,
-    SwitchV8,
-    CleanUp,
-  ]
 
-  RunScript(step_classes, config, options, side_effect_handler)
+  def _ProcessOptions(self, options):
+    if not options.manual and not options.reviewer:
+      print "A reviewer (-r) is required in (semi-)automatic mode."
+      return False
+    if not options.manual and not options.chromium:
+      print "A chromium checkout (-c) is required in (semi-)automatic mode."
+      return False
+    if not options.manual and not options.author:
+      print "Specify your chromium.org email with -a in (semi-)automatic mode."
+      return False
 
+    options.tbr_commit = not options.manual
+    return True
 
-def BuildOptions():
+  def _Steps(self):
-  parser = argparse.ArgumentParser()
+    return [
-  group = parser.add_mutually_exclusive_group()
+      Preparation,
-  group.add_argument("-f", "--force",
+      FreshBranch,
-                     help="Don't prompt the user.",
+      DetectLastPush,
-                     default=False, action="store_true")
+      PrepareChangeLog,
-  group.add_argument("-m", "--manual",
+      EditChangeLog,
-                     help="Prompt the user at every important step.",
+      IncrementVersion,
-                     default=False, action="store_true")
+      CommitLocal,
-  parser.add_argument("-a", "--author",
+      UploadStep,
-                      help="The author email used for rietveld.")
+      CommitRepository,
-  parser.add_argument("-b", "--last-bleeding-edge",
+      StragglerCommits,
-                      help=("The git commit ID of the last bleeding edge "
+      SquashCommits,
-                            "revision that was pushed to trunk. This is used "
+      NewBranch,
-                            "for the auto-generated ChangeLog entry."))
+      ApplyChanges,
-  parser.add_argument("-c", "--chromium",
+      SetVersion,
-                      help=("The path to your Chromium src/ directory to "
+      CommitTrunk,
-                            "automate the V8 roll."))
+      SanityCheck,
-  parser.add_argument("-l", "--last-push",
+      CommitSVN,
-                      help="The git commit ID of the last push to trunk.")
+      TagRevision,
-  parser.add_argument("-r", "--reviewer",
+      CheckChromium,
-                      help="The account name to be used for reviews.")
+      SwitchChromium,
-  parser.add_argument("-s", "--step",
+      UpdateChromiumCheckout,
-                      help="The step where to start work. Default: 0.",
+      UploadCL,
-                      default=0, type=int)
+      SwitchV8,
-  return parser
+      CleanUp,
+    ]
 
 
-def ProcessOptions(options):
-  if options.step < 0:
-    print "Bad step number %d" % options.step
-    return False
-  if not options.manual and not options.reviewer:
-    print "A reviewer (-r) is required in (semi-)automatic mode."
-    return False
-  if not options.manual and not options.chromium:
-    print "A chromium checkout (-c) is required in (semi-)automatic mode."
-    return False
-  if not options.manual and not options.author:
-    print "Specify your chromium.org email with -a in (semi-)automatic mode."
-    return False
-  return True
-
-
-def Main():
-  parser = BuildOptions()
-  options = parser.parse_args()
-  if not ProcessOptions(options):
-    parser.print_help()
-    return 1
-  RunPushToTrunk(CONFIG, PushToTrunkOptions(options))
-
 if __name__ == "__main__":
-  sys.exit(Main())
+  sys.exit(PushToTrunk(CONFIG).Run())

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
@@ -72,25 +71,6 @@ AUTO_ROLL_ARGS = [
 ]
 
 
-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.step = s
-  options.last_push = l
-  options.force = f
-  options.manual = m
-  options.reviewer = r
-  options.chromium = c
-  options.author = a
-  options.push = p
-  options.status_password = status_password
-  options.revert_bleeding_edge = revert_bleeding_edge
-  return options
-
-
 class ToplevelTest(unittest.TestCase):
   def testMakeComment(self):
     self.assertEquals("#   Line 1\n#   Line 2\n#",
@@ -302,13 +282,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 = options or CommonOptions(MakeOptions())
+    options = ScriptsBase(TEST_CONFIG, self, self._state).MakeOptions([])
-    state = state if state is not None else self._state
+    return MakeStep(step_class=Step, state=self._state,
-    return MakeStep(step_class=step_class, number=0, state=state,
+                    config=TEST_CONFIG, side_effect_handler=self,
-                    config=TEST_CONFIG, options=options,
+                    options=options)
-                    side_effect_handler=self)
+
+  def RunStep(self, script=PushToTrunk, step_class=Step, args=None):
+    """Convenience wrapper."""
+    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)
@@ -490,7 +474,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])
 
@@ -537,7 +521,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]))
@@ -550,7 +534,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"])
@@ -586,7 +570,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])
@@ -751,8 +735,7 @@ Performance and stability improvements on all platforms.""", commit)
     if force: args.append("-f")
     if manual: args.append("-m")
     else: args += ["-r", "reviewer@chromium.org"]
-    options = push_to_trunk.BuildOptions().parse_args(args)
+    PushToTrunk(TEST_CONFIG, self).Run(args)
-    RunPushToTrunk(TEST_CONFIG, PushToTrunkOptions(options), self)
 
     deps = FileToText(TEST_CONFIG[DEPS_FILE])
     self.assertTrue(re.search("\"v8_revision\": \"123456\"", deps))
@@ -781,9 +764,10 @@ Performance and stability improvements on all platforms.""", commit)
       ["svn log -1 --oneline ChangeLog", "r99 | Prepare push to trunk..."],
     ])
 
-    state = {}
+    self.RunStep(auto_roll.AutoRoll, FetchLatestRevision, AUTO_ROLL_ARGS)
-    self.MakeStep(FetchLatestRevision, state=state).Run()
+    self.assertRaises(Exception, lambda: self.RunStep(auto_roll.AutoRoll,
-    self.assertRaises(Exception, self.MakeStep(CheckLastPush, state=state).Run)
+                                                      CheckLastPush,
+                                                      AUTO_ROLL_ARGS))
 
   def testAutoRoll(self):
     password = self.MakeEmptyTempFile()
@@ -816,9 +800,8 @@ Performance and stability improvements on all platforms.""", commit)
       ["svn find-rev push_hash", "65"],
     ])
 
-    options = auto_roll.BuildOptions().parse_args(
+    auto_roll.AutoRoll(TEST_CONFIG, self).Run(
         AUTO_ROLL_ARGS + ["--status-password", password])
-    auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(options), self)
 
     state = json.loads(FileToText("%s-state.json"
                                   % TEST_CONFIG[PERSISTFILE_BASENAME]))
@@ -839,9 +822,8 @@ Performance and stability improvements on all platforms.""", commit)
       ["svn fetch", ""],
     ])
 
-    options = auto_roll.BuildOptions().parse_args(AUTO_ROLL_ARGS)
     def RunAutoRoll():
-      auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(options), self)
+      auto_roll.AutoRoll(TEST_CONFIG, self).Run(AUTO_ROLL_ARGS)
     self.assertRaises(Exception, RunAutoRoll)
 
   def testAutoRollStoppedByTreeStatus(self):
@@ -859,9 +841,8 @@ Performance and stability improvements on all platforms.""", commit)
       ["svn fetch", ""],
     ])
 
-    options = auto_roll.BuildOptions().parse_args(AUTO_ROLL_ARGS)
     def RunAutoRoll():
-      auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(options), self)
+      auto_roll.AutoRoll(TEST_CONFIG, self).Run(AUTO_ROLL_ARGS)
     self.assertRaises(Exception, RunAutoRoll)
 
   def testMergeToBranch(self):
@@ -982,24 +963,19 @@ LOG=N
     # ports of r12345. r56789 is the MIPS port of r34567.
     args = ["-f", "-p", extra_patch, "--branch", "trunk", "12345", "23456",
             "34567"]
-    options = merge_to_branch.BuildOptions().parse_args(args)
-    self.assertTrue(merge_to_branch.ProcessOptions(options))
 
     # The first run of the script stops because of the svn being down.
     self.assertRaises(GitFailedException,
-        lambda: RunMergeToBranch(TEST_CONFIG,
+        lambda: MergeToBranch(TEST_CONFIG, self).Run(args))
-                                 MergeToBranchOptions(options),
-                                 self))
 
     # Test that state recovery after restarting the script works.
-    options.step = 3
+    args += ["-s", "3"]
-    RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options), self)
+    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(
 """------------------------------------------------------------------------