Closed
Bug 1193264
Opened 9 years ago
Closed 9 years ago
Allow saving and using preset try strings from |mach try|
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox44 fixed)
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: jgraham, Unassigned)
References
Details
Attachments
(1 file)
e.g. ./mach try -b d -p linux,linux64 -u mochitest --save mochitest-linux-debug
./mach try --preset mochitest-linux-debug
Reporter | ||
Comment 1•9 years ago
|
||
Bug 1193264 - Add support for saving and reusing try strings in mach try
Adds --save and --preset arguments that can be used to store and reuse
frequently used try strings.
Attachment #8646454 -
Flags: review?(cmanchester)
Reporter | ||
Comment 2•9 years ago
|
||
Reporter | ||
Comment 3•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
Bug 1193264 - Add support for saving and reusing try strings in mach try
Adds --save and --preset arguments that can be used to store and reuse
frequently used try strings.
Reporter | ||
Comment 4•9 years ago
|
||
Reporter | ||
Comment 5•9 years ago
|
||
Reporter | ||
Comment 6•9 years ago
|
||
Reporter | ||
Comment 7•9 years ago
|
||
Reporter | ||
Comment 8•9 years ago
|
||
Reporter | ||
Comment 9•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
Bug 1193264 - Add support for saving and reusing try strings in mach try
Adds --save and --preset arguments that can be used to store and reuse
frequently used try strings.
Comment 10•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
https://reviewboard.mozilla.org/r/15777/#review15787
::: testing/mach_commands.py:397
(Diff revision 3)
> + for x in item.split(","):
> + if x.strip():
[x for x in item.split(",") if x.strip()] ?
::: testing/mach_commands.py:403
(Diff revision 3)
> + if not len(kwargs["paths"]) and not kwargs["tests"]:
> + print("Paths or tests must be specified as an argument to autotry.")
This didn't get an update for the option to only provide tags.
::: testing/mach_commands.py:435
(Diff revision 3)
> - def autotry(self, builds=None, platforms=None, paths=None, verbose=None,
> + def autotry(self, **kwargs):
> - push=None, tags=None, tests=None, extra_args=None):
I would prefer to keep individual parameters.
::: testing/mach_commands.py:481
(Diff revision 3)
> + defaults = at.load_config(kwargs["_load"])
> +
> + if defaults is None:
> + print("No saved configuration called %s found in mach.ini" % kwargs["_load"],
> + file=sys.stderr)
kwargs["load"]
::: testing/tools/autotry/autotry.py:32
(Diff revision 3)
> + parser.add_argument('--save', dest="save", action='store',
> + help="Save the command line arguments for future use with --preset")
> + parser.add_argument('--preset', dest="load", action='store',
> + help="Load a saved set of arguments. Additional arguments will override saved ones")
A nice feature for a follow up would be to have the final result of the last N invocations of mach try saved automatically, with a way to dump those to the console and select by index.
::: testing/tools/autotry/autotry.py:32
(Diff revision 3)
> + parser.add_argument('--save', dest="save", action='store',
> + help="Save the command line arguments for future use with --preset")
> + parser.add_argument('--preset', dest="load", action='store',
> + help="Load a saved set of arguments. Additional arguments will override saved ones")
A nice feature for a follow up would be to have the final result of the last N invocations of mach try saved automatically, with a way to dump those to the console and select by index.
::: testing/tools/autotry/autotry.py:82
(Diff revision 3)
> + config_file = os.path.join(self.topsrcdir, "mach.ini")
I think we can use mach_context.state_dir for this.
::: testing/tools/autotry/autotry.py:95
(Diff revision 3)
> + for item in kwargs.keys():
> + if item.startswith("_"):
> + kwargs.pop(item)
I don't think this underscore prefix is relevant anymore.
Attachment #8646454 -
Flags: review?(cmanchester)
Reporter | ||
Comment 11•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
Bug 1193264 - Add support for saving and reusing try strings in mach try
Adds --save and --preset arguments that can be used to store and reuse
frequently used try strings.
Attachment #8646454 -
Flags: review?(cmanchester)
Updated•9 years ago
|
Attachment #8646454 -
Flags: review?(cmanchester)
Comment 12•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
https://reviewboard.mozilla.org/r/15777/#review17109
This doesn't look ready for re-review.
Reporter | ||
Comment 13•9 years ago
|
||
https://reviewboard.mozilla.org/r/15777/#review15787
> [x for x in item.split(",") if x.strip()] ?
Fixed, in the sense that I suddenly realised that I actually use the test[platform] syntax, so I implemented that too.
> I would prefer to keep individual parameters.
Unfortunately that doesn't work with the --load feature; it needs a dictionary of arguments that can be updated. Of course I could make this work with explicitly named arguments, but it would involve a lot of repetitive code, so I don't think it's worthwhile.
Reporter | ||
Updated•9 years ago
|
Attachment #8646454 -
Flags: review?(cmanchester)
Reporter | ||
Comment 14•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
Bug 1193264 - Add support for saving and reusing try strings in mach try
Adds --save and --preset arguments that can be used to store and reuse
frequently used try strings.
Comment 15•9 years ago
|
||
https://reviewboard.mozilla.org/r/15777/#review15787
> Unfortunately that doesn't work with the --load feature; it needs a dictionary of arguments that can be updated. Of course I could make this work with explicitly named arguments, but it would involve a lot of repetitive code, so I don't think it's worthwhile.
There are several values to provide as defaults -- updating a dictionary is hardly the only reasonable implementation, but I guess this is ok.
Comment 16•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
https://reviewboard.mozilla.org/r/15777/#review17321
::: testing/mach_commands.py:517
(Diff revision 5)
> + print("No saved configuration called %s found in mach.ini" % kwargs["load"],
autotry.ini now
::: testing/tools/autotry/autotry.py:63
(Diff revision 5)
> +class TryArgumentParser(object):
> + """Simple three-state parser for handling expressions
> + of the from "foo[sub item, another], bar,baz". This takes
> + input from the TryArgumentTokenizer and runs through a small
> + state machine, returning a dictionary of {top-level-item:[sub_items]}
> + i.e. the above would result in
> + {"foo":["sub item", "another"], "bar": [], "baz": []}
> + In the case of invalid input a ValueError is raised."""
Reinventing the try parser here really seems like overkill -- where are we actually interacting with individual platform filters? Couldn't we just serialize the result of parsing suites/filters in a well known format and convert it to try syntax only when pushing?
Attachment #8646454 -
Flags: review?(cmanchester)
Comment 17•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
https://reviewboard.mozilla.org/r/15777/#review17333
::: testing/tools/autotry/autotry.py:101
(Diff revision 5)
> + raise ValueError("Error parsing try string, unexpected" % (self.token[0]))
There's no format pattern in this string.
::: testing/tools/autotry/autotry.py:203
(Diff revision 5)
> + kwargs = vars(parser().parse_args(data.split()))
"parser" undefined?
Attachment #8646454 -
Flags: review+
Reporter | ||
Comment 18•9 years ago
|
||
https://reviewboard.mozilla.org/r/15777/#review17333
> "parser" undefined?
No, but I tried to make the name more obvious.
Reporter | ||
Comment 19•9 years ago
|
||
Comment on attachment 8646454 [details]
MozReview Request: Bug 1193264 - Add support for saving and reusing try strings in mach try
Bug 1193264 - Add support for saving and reusing try strings in mach try
Adds --save and --preset arguments that can be used to store and reuse
frequently used try strings.
Comment 20•9 years ago
|
||
Comment 21•9 years ago
|
||
sorry had to back this out since one of the changes caused https://treeherder.mozilla.org/logviewer.html#?job_id=14640968&repo=mozilla-inbound
Comment 22•9 years ago
|
||
Comment 23•9 years ago
|
||
Comment 24•9 years ago
|
||
backed out for the test bustage in bug 1193215
Comment 25•9 years ago
|
||
Comment 26•9 years ago
|
||
Comment 27•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in
before you can comment on or make changes to this bug.
Description
•