Reviewers: ulan,
Message:
PTAL
Description:
Add named options to push-to-trunk script.
Also make sure that on exceptions from the test infrastructure there is no
retry.
BUG=
Please review this at https://codereview.chromium.org/114203002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+57, -28 lines):
M tools/push-to-trunk/auto_roll.py
M tools/push-to-trunk/common_includes.py
M tools/push-to-trunk/push_to_trunk.py
M tools/push-to-trunk/test_scripts.py
Index: tools/push-to-trunk/auto_roll.py
diff --git a/tools/push-to-trunk/auto_roll.py
b/tools/push-to-trunk/auto_roll.py
index
8f26a49cb3ca9dcfd99f08080aa9f02aca72e4da..c7de2a7067554e1473baa2b996a6af67405c59a5
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())
Index: tools/push-to-trunk/common_includes.py
diff --git a/tools/push-to-trunk/common_includes.py
b/tools/push-to-trunk/common_includes.py
index
8d08f30ca4042efedf46273fbe85f6d39940ad62..e4a04002e6625d7a1a143be6c700e644bdf9dea0
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.
Index: tools/push-to-trunk/push_to_trunk.py
diff --git a/tools/push-to-trunk/push_to_trunk.py
b/tools/push-to-trunk/push_to_trunk.py
index
545cc7579dd89f621c867af06750e50122da97c9..ac999fcdccc895efd522fe50199142c9204f4238
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())
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
427c3b2f283c21358ebf68e503c24f571dc33d35..cd65a2bdbbb65f3efe9b0aefc36f1ed3d953b546
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="[email protected]" 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(
"""------------------------------------------------------------------------
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
