Move builders.py and steps.py to chromium_tests.

Move builders.py and steps.py to chromium_tests.

BUG=472611
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=295678

[Adrian Kuegel, 2015-06-12 14:11:32]

akuegel@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Adrian Kuegel, 2015-06-12 14:11:33]

Pawel, can you please review this CL?
With this change, we can explicitly tell which builders config should be loaded.
In that case, I think it would be actually fine if other teams also want to add
their builder configs, we just explicitly only load our configs for chromium.

[Adrian Kuegel, 2015-06-12 15:10:10]

On 2015/06/12 14:11:33, Adrian Kuegel wrote:
> Pawel, can you please review this CL?
> With this change, we can explicitly tell which builders config should be
loaded.
> In that case, I think it would be actually fine if other teams also want to
add
> their builder configs, we just explicitly only load our configs for chromium.

Btw, the latest patchset (almost) passes presubmits, the only thing failing is
that coverage seems to be missing for some v8 config:
scripts/slave/recipe_modules/v8/config                                    59    
 2    97%   83-84
Not sure how/why my change affects this.

[Paweł Hajdan Jr., 2015-06-15 09:09:29]

Thanks for the patch. I think it's a good direction in general.

I'd still prefer to move to other recipes storing their bot configs inline
rather than in the shared location.

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/api.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/api.py:25: def
load_builder_configs(self, mastername):
Why do we need this method? Wasn't this done implicitly in the chromium recipe
module before?

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/api.py:26:
self.m.chromium.add_builders({mastername: builders.BUILDERS[mastername]})
Do we still need chromium.add_builders? Do you plan to remove it?

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/a...
File scripts/slave/recipes/android/perf.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/a...
scripts/slave/recipes/android/perf.py:106:
api.chromium_tests.load_builder_configs(mastername)
This recipe should be using its own configs rather than the shared ones. Could
you add at least a TODO to clean this up?

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisect.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
scripts/slave/recipes/bisect.py:236:
api.chromium_tests.load_builder_configs(mastername)
This recipe should be using its own configs rather than the shared ones. Could
you add at least a TODO to clean this up?

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/desktop_bisect.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.py:15:
api.chromium_tests.load_builder_configs(mastername)
This recipe should be using its own configs rather than the shared ones. Could
you add at least a TODO to clean this up?

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/c...
File scripts/slave/recipes/chromium.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.py:56: # TODO(akuegel): Filter out those masters
that are not using this recipe.
Well, I'd say these masters just shouldn't be in chromium_tests.builders then.

[Adrian Kuegel, 2015-06-15 12:33:22]

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/api.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/api.py:25: def
load_builder_configs(self, mastername):
On 2015/06/15 09:09:29, Paweł Hajdan Jr. wrote:
> Why do we need this method? Wasn't this done implicitly in the chromium recipe
> module before?

As we discussed offline, I will preserve what we currently have, so I removed
this function again and initialized the builders in the constructor.

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/api.py:26:
self.m.chromium.add_builders({mastername: builders.BUILDERS[mastername]})
On 2015/06/15 09:09:29, Paweł Hajdan Jr. wrote:
> Do we still need chromium.add_builders? Do you plan to remove it?

I guess it makes sense to move also the builders property and the add_builders
function to here. I did that now. I also moved the two function in chromium
recipe module which directly or indirectly depended on that property to
chromium_tests to avoid a cyclic dependency.

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/a...
File scripts/slave/recipes/android/perf.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/a...
scripts/slave/recipes/android/perf.py:106:
api.chromium_tests.load_builder_configs(mastername)
On 2015/06/15 09:09:29, Paweł Hajdan Jr. wrote:
> This recipe should be using its own configs rather than the shared ones. Could
> you add at least a TODO to clean this up?

Done.

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisect.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
scripts/slave/recipes/bisect.py:236:
api.chromium_tests.load_builder_configs(mastername)
On 2015/06/15 09:09:29, Paweł Hajdan Jr. wrote:
> This recipe should be using its own configs rather than the shared ones. Could
> you add at least a TODO to clean this up?

Done.

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/desktop_bisect.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.py:15:
api.chromium_tests.load_builder_configs(mastername)
On 2015/06/15 09:09:29, Paweł Hajdan Jr. wrote:
> This recipe should be using its own configs rather than the shared ones. Could
> you add at least a TODO to clean this up?

Done.

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/c...
File scripts/slave/recipes/chromium.py (right):

https://codereview.chromium.org/1185693002/diff/40001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.py:56: # TODO(akuegel): Filter out those masters
that are not using this recipe.
On 2015/06/15 09:09:29, Paweł Hajdan Jr. wrote:
> Well, I'd say these masters just shouldn't be in chromium_tests.builders then.

Acknowledged. I removed this TODO.

[Paweł Hajdan Jr., 2015-06-15 14:17:12]

LGTM, thanks!

[Adrian Kuegel, 2015-06-15 14:22:24]

The CQ bit was checked by akuegel@chromium.org

[commit-bot: I haz the power, 2015-06-15 14:22:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185693002/60001

[commit-bot: I haz the power, 2015-06-15 14:26:29]

Committed patchset #4 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=295678

[smut, 2015-06-15 21:13:21]

smut@google.com changed reviewers:
+ smut@google.com

[smut, 2015-06-15 21:13:22]

FYI I think this is causing build presubmit errors:

tests/masters_recipes_test.py (0.11s) failed
Traceback (most recent call last):
  File "tests/masters_recipes_test.py", line 23, in <module>
    from chromium.builders import BUILDERS
ImportError: No module named builders

[Dirk Pranke, 2015-06-15 21:33:04]

On 2015/06/15 21:13:22, smut wrote:
> FYI I think this is causing build presubmit errors:
> 
> tests/masters_recipes_test.py (0.11s) failed
> Traceback (most recent call last):
>   File "tests/masters_recipes_test.py", line 23, in <module>
>     from chromium.builders import BUILDERS
> ImportError: No module named builders

I am also seeing this.

[pgervais, 2015-06-15 22:44:12]

On 2015/06/15 21:33:04, Dirk Pranke wrote:
> On 2015/06/15 21:13:22, smut wrote:
> > FYI I think this is causing build presubmit errors:
> > 
> > tests/masters_recipes_test.py (0.11s) failed
> > Traceback (most recent call last):
> >   File "tests/masters_recipes_test.py", line 23, in <module>
> >     from chromium.builders import BUILDERS
> > ImportError: No module named builders
> 
> I am also seeing this.

Fix here: https://chromiumcodereview.appspot.com/1185193006/