Closed Bug 1531886 Opened 8 months ago Closed 7 months ago

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


(Firefox Build System :: Try, enhancement)

Not set


(firefox67 fixed)

Tracking Status
firefox67 --- fixed


(Reporter: ahal, Assigned: ahal)




(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:

    - 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:

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

This allows us to refactor 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 '' function in, 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 will error if someone makes a typo in an
in-tree preset.

Depends on D22023

Pushed by
[tryselect] Use consistent 'run' method to kickstart all selectors, r=gbrown
[tryselect] Handle templates in, r=gbrown
[tryselect] Delete template context from kwargs, r=gbrown
[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.