Closed Bug 1391675 Opened 2 years ago Closed 2 years ago

[tryselect] Stand up some tests for |mach try|

Categories

(Testing :: General, enhancement)

enhancement
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(5 files)

There isn't any way to test changes to |mach try| at the moment. I think something like this would be best tested with the 'cram' test framework (aka mercurial .t tests).
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment on attachment 8899602 [details]
Bug 1391675 - [tryselect] Make main command and all subcommands use the same argument parser,

https://reviewboard.mozilla.org/r/170906/#review176260

Nice to see things more properly structured. Good job Andrew.
Attachment #8899602 - Flags: review?(armenzg) → review+
Comment on attachment 8899603 [details]
Bug 1391675 - [tryselect] Move --no-push into common arguments,

https://reviewboard.mozilla.org/r/170908/#review176264
Attachment #8899603 - Flags: review?(armenzg) → review+
Comment on attachment 8899601 [details]
Bug 1391675 - [mach] Allow commands to have both a parser and subcommands,

https://reviewboard.mozilla.org/r/170904/#review176842

Yes, mach shouldn't get in the way here.

I'm wondering if we should verify the parser doesn't have positional arguments though - as I would expect those to interfere with sub-command handling. But if someone shoots themselves in the foot, then they shoot themselves in the foot :)
Attachment #8899601 - Flags: review?(gps) → review+
Comment on attachment 8899604 [details]
Bug 1391675 - [tryselect] Add a test for 'mach try fuzzy' and associated task,

https://reviewboard.mozilla.org/r/170910/#review176854

Very nice!

::: taskcluster/docker/lint/system-setup.sh:63
(Diff revision 1)
> +
> +tooltool_fetch <<EOF
> +[
> +  {
> +    "size": 866160,
> +    "visibility": "public",

This line isn't needed for fetching.

::: tools/tryselect/test/test_fuzzy.t:10
(Diff revision 1)
> +
> +  $ ./mach try fuzzy --no-push -q "'foo"
> +  Calculated try selector:
> +  {
> +    "tasks": [
> +      "test/foo-debug", 

Pro tip: If you pass ``separators=(',', ':')`` to ``json.dump`` or ``json.dumps`` this trailing whitespace goes away. I find myself doing a lot of that in version-control-tools to make test output more sane.
Attachment #8899604 - Flags: review?(gps) → review+
Comment on attachment 8899601 [details]
Bug 1391675 - [mach] Allow commands to have both a parser and subcommands,

https://reviewboard.mozilla.org/r/170904/#review176842

Surprisingly that case also seems to work as expected. If the positional arg matches a subcommand, it dispatches and if not, it is passed through like normal.
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6bb409c7fc4
[mach] Allow commands to have both a parser and subcommands, r=gps
https://hg.mozilla.org/integration/autoland/rev/6f29f12cd26d
[tryselect] Make main command and all subcommands use the same argument parser, r=armenzg
https://hg.mozilla.org/integration/autoland/rev/a2a371e7e6f8
[tryselect] Move --no-push into common arguments, r=armenzg
https://hg.mozilla.org/integration/autoland/rev/c4fc5865ac38
[tryselect] Add a test for 'mach try fuzzy' and associated task, r=gps
Attachment #8900416 - Flags: review?(gps)
Comment on attachment 8900416 [details]
Bug 1391675 - Fix sm-pkg bustage on a CLOSED TREE,

https://reviewboard.mozilla.org/r/171762/#review176984
Attachment #8900416 - Flags: review?(gps) → review+
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f6306658731b
Fix sm-pkg bustage on a CLOSED TREE, r=bustage CLOSED TREE
You need to log in before you can comment on or make changes to this bug.