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)

defect
Not set
normal

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
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)
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 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 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)
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)
Blocks: 1204120
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.
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.
Attachment #8646454 - Flags: review?(cmanchester)
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.
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 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 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+
https://reviewboard.mozilla.org/r/15777/#review17333 > "parser" undefined? No, but I tried to make the name more obvious.
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.
backed out for the test bustage in bug 1193215
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: