From 508f7d256a120fdb1e5ef5b653d66e7d49232e98 Mon Sep 17 00:00:00 2001 From: "machenbach@chromium.org" Date: Mon, 16 Dec 2013 10:56:52 +0000 Subject: [PATCH] Add named options to push-to-trunk script. Also make sure that on exceptions from the test infrastructure there is no retry. BUG= R=ulan@chromium.org Review URL: https://codereview.chromium.org/114203002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18313 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- tools/push-to-trunk/auto_roll.py | 8 +++++++- tools/push-to-trunk/common_includes.py | 34 ++++++++++++++++++++++------------ tools/push-to-trunk/push_to_trunk.py | 18 ++++++++++++++---- tools/push-to-trunk/test_scripts.py | 25 ++++++++++++++----------- 4 files changed, 57 insertions(+), 28 deletions(-) diff --git a/tools/push-to-trunk/auto_roll.py b/tools/push-to-trunk/auto_roll.py index 8f26a49..c7de2a7 100755 --- a/tools/push-to-trunk/auto_roll.py +++ b/tools/push-to-trunk/auto_roll.py @@ -38,6 +38,12 @@ CONFIG = { } +class AutoRollOptions(CommonOptions): + def __init__(self, options): + super(AutoRollOptions, self).__init__(options) + self.requires_editor = False + + class Preparation(Step): MESSAGE = "Preparation." @@ -137,7 +143,7 @@ def Main(): print "You need to specify the chromium src location and a reviewer." parser.print_help() return 1 - RunAutoRoll(CONFIG, options) + RunAutoRoll(CONFIG, AutoRollOptions(options)) if __name__ == "__main__": sys.exit(Main()) diff --git a/tools/push-to-trunk/common_includes.py b/tools/push-to-trunk/common_includes.py index 8d08f30..e4a0400 100644 --- a/tools/push-to-trunk/common_includes.py +++ b/tools/push-to-trunk/common_includes.py @@ -212,6 +212,19 @@ class SideEffectHandler(object): DEFAULT_SIDE_EFFECT_HANDLER = SideEffectHandler() +class NoRetryException(Exception): + pass + +class CommonOptions(object): + def __init__(self, options, manual=True): + self.requires_editor = True + self.wait_for_lgtm = True + self.s = options.s + self.force_readline_defaults = not manual + self.force_upload = not manual + self.manual = manual + + class Step(object): def __init__(self, text, requires, number, config, state, options, handler): self._text = text @@ -225,16 +238,11 @@ class Step(object): 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 Config(self, key): return self._config[key] - def IsForced(self): - return self._options and self._options.f - - def IsManual(self): - return self._options and self._options.m - def Run(self): if self._requires: self.RestoreIfUnset(self._requires) @@ -263,6 +271,8 @@ class Step(object): got_exception = False try: result = cb() + except NoRetryException, e: + raise e except Exception: got_exception = True if got_exception or retry_on(result): @@ -277,7 +287,7 @@ class Step(object): def ReadLine(self, default=None): # Don't prompt in forced mode. - if not self.IsManual() and default is not None: + if self._options.force_readline_defaults and default is not None: print "%s (forced)" % default return default else: @@ -288,7 +298,7 @@ class Step(object): return self.Retry(cmd, retry_on, [5, 30]) def Editor(self, args): - if not self.IsForced(): + if self._options.requires_editor: return self._side_effect_handler.Command(os.environ["EDITOR"], args, pipe=False) @@ -307,7 +317,7 @@ class Step(object): raise Exception(msg) def DieNoManualMode(self, msg=""): - if not self.IsManual(): + if not self._options.manual: msg = msg or "Only available in manual mode." self.Die(msg) @@ -348,7 +358,7 @@ class Step(object): self.Die("This is not a git checkout, this script won't work for you.") # Cancel if EDITOR is unset or not executable. - if (not self.IsForced() and (not os.environ.get("EDITOR") or + if (self._options.requires_editor and (not os.environ.get("EDITOR") or Command("which", os.environ["EDITOR"]) is None)): self.Die("Please set your EDITOR environment variable, you'll need it.") @@ -418,7 +428,7 @@ class Step(object): answer = "" while answer != "LGTM": print "> ", - answer = self.ReadLine("LGTM" if self.IsForced() else None) + answer = self.ReadLine(None if self._options.wait_for_lgtm else "LGTM") if answer != "LGTM": print "That was not 'LGTM'." @@ -454,7 +464,7 @@ class UploadStep(Step): print "Please enter the email address of a V8 reviewer for your patch: ", self.DieNoManualMode("A reviewer must be specified in forced mode.") reviewer = self.ReadLine() - force_flag = " -f" if not self.IsManual() else "" + force_flag = " -f" if self._options.force_upload else "" args = "cl upload -r \"%s\" --send-mail%s" % (reviewer, force_flag) # TODO(machenbach): Check output in forced mode. Verify that all required # base files were uploaded, if not retry. diff --git a/tools/push-to-trunk/push_to_trunk.py b/tools/push-to-trunk/push_to_trunk.py index 545cc75..ac999fc 100755 --- a/tools/push-to-trunk/push_to_trunk.py +++ b/tools/push-to-trunk/push_to_trunk.py @@ -52,6 +52,16 @@ CONFIG = { } +class PushToTrunkOptions(CommonOptions): + def __init__(self, options): + super(PushToTrunkOptions, self).__init__(options, options.m) + self.requires_editor = not options.f + self.wait_for_lgtm = not options.f + self.tbr_commit = not options.m + self.l = options.l + self.r = options.r + self.c = options.c + class Preparation(Step): MESSAGE = "Preparation." @@ -214,7 +224,7 @@ class CommitLocal(Step): # Include optional TBR only in the git command. The persisted commit # message is used for finding the commit again later. - review = "\n\nTBR=%s" % self._options.r if not self.IsManual() else "" + review = "\n\nTBR=%s" % self._options.r if self._options.tbr_commit else "" if self.Git("commit -a -m \"%s%s\"" % (prep_commit_msg, review)) is None: self.Die("'git commit -a' failed.") @@ -441,7 +451,7 @@ class UploadCL(Step): ver = "%s.%s.%s" % (self._state["major"], self._state["minor"], self._state["build"]) - if self._options and self._options.r: + if self._options.r: print "Using account %s for review." % self._options.r rev = self._options.r else: @@ -451,7 +461,7 @@ class UploadCL(Step): args = "commit -am \"Update V8 to version %s.\n\nTBR=%s\"" % (ver, rev) if self.Git(args) is None: self.Die("'git commit' failed.") - force_flag = " -f" if not self.IsManual() else "" + force_flag = " -f" if self._options.force_upload else "" if self.Git("cl upload --send-mail%s" % force_flag, pipe=False) is None: self.Die("'git cl upload' failed, please try again.") print "CL uploaded." @@ -569,7 +579,7 @@ def Main(): if not ProcessOptions(options): parser.print_help() return 1 - RunPushToTrunk(CONFIG, options) + RunPushToTrunk(CONFIG, PushToTrunkOptions(options)) if __name__ == "__main__": sys.exit(Main()) diff --git a/tools/push-to-trunk/test_scripts.py b/tools/push-to-trunk/test_scripts.py index 427c3b2..cd65a2b 100644 --- a/tools/push-to-trunk/test_scripts.py +++ b/tools/push-to-trunk/test_scripts.py @@ -35,8 +35,9 @@ from common_includes import * import push_to_trunk from push_to_trunk import * import auto_roll -from auto_roll import FetchLatestRevision +from auto_roll import AutoRollOptions from auto_roll import CheckLastPush +from auto_roll import FetchLatestRevision TEST_CONFIG = { @@ -230,14 +231,14 @@ class SimpleMock(object): # 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.") + raise NoRetryException("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)) + raise NoRetryException("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. @@ -252,8 +253,8 @@ class SimpleMock(object): 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))) + raise NoRetryException("Called %s too seldom: %d vs. %d" + % (self._name, self._index, len(self._recipe))) class ScriptTest(unittest.TestCase): @@ -278,7 +279,7 @@ class ScriptTest(unittest.TestCase): def MakeStep(self, step_class=Step, state=None, options=None): """Convenience wrapper.""" - options = options or MakeOptions() + options = options or CommonOptions(MakeOptions()) return MakeStep(step_class=step_class, number=0, state=state, config=TEST_CONFIG, options=options, side_effect_handler=self) @@ -710,7 +711,7 @@ Performance and stability improvements on all platforms.""" options = MakeOptions(f=force, m=manual, r="reviewer@chromium.org" if not manual else None, c = TEST_CONFIG[CHROMIUM]) - RunPushToTrunk(TEST_CONFIG, options, self) + RunPushToTrunk(TEST_CONFIG, PushToTrunkOptions(options), self) deps = FileToText(TEST_CONFIG[DEPS_FILE]) self.assertTrue(re.search("\"v8_revision\": \"123456\"", deps)) @@ -759,7 +760,9 @@ Performance and stability improvements on all platforms.""" ["svn log -1 --oneline ChangeLog", "r65 | Prepare push to trunk..."], ]) - auto_roll.RunAutoRoll(TEST_CONFIG, MakeOptions(m=False, f=True), self) + auto_roll.RunAutoRoll(TEST_CONFIG, + AutoRollOptions(MakeOptions(m=False, f=True)), + self) self.assertEquals("100", self.MakeStep().Restore("lkgr")) self.assertEquals("101", self.MakeStep().Restore("latest")) @@ -768,7 +771,7 @@ Performance and stability improvements on all platforms.""" class SystemTest(unittest.TestCase): def testReload(self): step = MakeStep(step_class=PrepareChangeLog, number=0, state={}, config={}, - options=None, + options=CommonOptions(MakeOptions()), side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER) body = step.Reload( """------------------------------------------------------------------------ -- 2.7.4