Closed Bug 1531886 Opened 8 months ago Closed 7 months ago

Presets with typos in them don't result in an error

Categories

(Firefox Build System :: Try, enhancement)

enhancement
Not set

Tracking

(firefox67 fixed)

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(4 files)

This is something I noticed while working on bug 1513951, but it was fairly late in the game and the fix was pretty complicated so I decided to leave it for a follow-up.

In that bug I added a test to make sure that all presets in tools/tryselect/try_presets.yml can actually run. And while that is true, typos in the values don't cause an error. For example:

queries:
    - mochitests

Will work, even though it actually needs to be named query. This wasn't a huge deal when presets were more or less only generated, but people might start handcrafting them for in-tree usage now.

Forgot to mention that the reason this doesn't fail, is that all of the selector's run methods accept **kwargs:
https://searchfox.org/mozilla-central/source/tools/tryselect/selectors/fuzzy.py#207

So typoed names end up in there and the default value gets used for the actual name instead.

This allows us to refactor mach_commands.py so we can call self.handle_presets
implicitly. This in turn gives us a future place to add shared code and makes
adding new selectors easier.

This was previously only in the cli parser because that was the only shared
place that ran for all selectors. Now that we have the 'self.run' function in
mach_commands.py, we can move it there. This move is also needed to allow us to
remove 'templates' from kwargs (which happens in the next commit).

Depends on D22021

When we parse template arguments, we stuff them all in kwargs['templates'],
however we don't delete the old argument. This results in all kinds of unused
variables lying around in kwargs. E.g we would have both
kwargs['templates']['env'] and kwargs['env'] (the latter being unused). This is
the main reason why all the selector's run functions need to have a **kwargs at
the end of them.

Depends on D22022

This forces us to be more strict with what gets passed into the try selector.
This change means test_preset.py will error if someone makes a typo in an
in-tree preset.

Depends on D22023

Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bee5219e28b2
[tryselect] Use consistent 'run' method to kickstart all selectors, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/ba832d587ae1
[tryselect] Handle templates in mach_commands.py, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/ac6bdcb49765
[tryselect] Delete template context from kwargs, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/b5d943805f64
[tryselect] Stop allowing **kwargs in subcommand run functions, r=gbrown
Depends on: 1533474
You need to log in before you can comment on or make changes to this bug.