chromium_tests: require explicit compile targets (except for Android)

chromium_tests: require explicit compile targets (except for Android)

BUG=536186

[Paweł Hajdan Jr., 2015-11-23 15:28:09]

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

[Nico, 2015-11-23 16:20:04]

The recipe code change is fine, but I'd like to understand a bit better under
which conditions the error message shows up in the golden files. I think it'd be
nice if we could somehow arrive at a state where the error makes it into no
golden files, at least for the production masters (chromium_perf etc --
chromium_fyi isn't a production master)

For example, why does full_chromium_perf_Android_Builder get this error but none
of the other perf bots do?

https://codereview.chromium.org/1472893002/diff/1/scripts/slave/recipes/chrom...
File scripts/slave/recipes/chromium.expected/full_chromium_Linux_x64.json
(right):

https://codereview.chromium.org/1472893002/diff/1/scripts/slave/recipes/chrom...
scripts/slave/recipes/chromium.expected/full_chromium_Linux_x64.json:162:
"@@@STEP_TEXT@Please add explicit compile targets for the build@@@",
This is the main waterfall Linux x64 clobber bot, right? That should probably
get an explicit target 'all' (or 'gn_all') target first?


...oh, it looks like something does pass gn_all for this bot, I suppose
https://code.google.com/p/chromium/codesearch#chromium/src/testing/buildbot/c...

So as long that is parsed before the recipes run I suppose things should be
fine.

But it looks like the testing logic does inject dummy test targets (see All and
chromium_builder tests in this file on the lhs, and see
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...
and
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...).
I suppose the test targets injected by src/testing/buildbot are processed
earlier? Do we still need default_targets in recipes after this change? It
shouldn't be used anywhere any more, right?

Why does Linux x64 get this error in its expectations file but Linux (the 32-bit
one) doesn't?

It's very very unfortunate that the golden files don't know about these targets
-- can you remind me where the "move recipes to src/" bug is again?

[Dirk Pranke, 2015-11-24 01:54:21]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2015-11-24 01:54:22]

no lgtm for the moment.

As I note below, I think we're not using very good default values for the step
test data for reading the bot config and as a result the expectations are
increasingly unreflective of what the bot will actually normally do.

I'll see if I can post a separate CL w/ a fix, and then maybe we can come back
to this?

https://codereview.chromium.org/1472893002/diff/1/scripts/slave/recipes/chrom...
File scripts/slave/recipes/chromium.expected/full_chromium_Linux_x64.json
(right):

https://codereview.chromium.org/1472893002/diff/1/scripts/slave/recipes/chrom...
scripts/slave/recipes/chromium.expected/full_chromium_Linux_x64.json:162:
"@@@STEP_TEXT@Please add explicit compile targets for the build@@@",
On 2015/11/23 16:20:04, Nico wrote:
> This is the main waterfall Linux x64 clobber bot, right? That should probably
> get an explicit target 'all' (or 'gn_all') target first?

I think the expectations are using the wrong test data at the moment, and as a
result
we're not really seeing the correct thing here. I think this got "broken" in 

https://codereview.chromium.org/1466563002/

and I need to fix it.

> 
> 
> ...oh, it looks like something does pass gn_all for this bot, I suppose
>
https://code.google.com/p/chromium/codesearch#chromium/src/testing/buildbot/c...
> 
> So as long that is parsed before the recipes run I suppose things should be
> fine.
> 
> But it looks like the testing logic does inject dummy test targets (see All
and
> chromium_builder tests in this file on the lhs, and see
>
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...
> and
>
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...).
> I suppose the test targets injected by src/testing/buildbot are processed
> earlier? Do we still need default_targets in recipes after this change? It
> shouldn't be used anywhere any more, right?

It is still used. I would like to get rid of these where possible and have the
targets only live src-side.

> 
> Why does Linux x64 get this error in its expectations file but Linux (the
32-bit
> one) doesn't?

Those have two different configs (Linux is still not using MB or GN).

> It's very very unfortunate that the golden files don't know about these
targets

Agreed I think we need to clean up the golden files before landing this.

> -- can you remind me where the "move recipes to src/" bug is again?

I believe that's crbug.com/459840.

[Dirk Pranke, 2016-08-31 19:02:52]

Description was changed from

==========
chromium_tests: require explicit compile targets (except for Android)

BUG=536186
==========

to

==========
chromium_tests: require explicit compile targets (except for Android)

BUG=536186
==========

[Dirk Pranke, 2016-08-31 19:03:22]

closing this as I don't think we ever reached a consensus here and the current
approach would probably want to be different regardless.