+jbudorick, dpranke

How often do we get such breakages?

It is hard to tell but I would guess once a month.

Putting builders in the CQ is comparatively expensive (hundreds of builds a day), so once a month is on the order of 1 out of every 10,000 builds. Which probably isn't worth it. 

However, I'm still trying to figure out the right cost model here, so we should probably keep this open for now.

We already have the compile-only builder running, though. If we expand that to a tester & launch the tests into the N5 pool once it expands, we might be able to do this at a lower marginal cost.

oh, sorry, I missed that this was just asking for tests, I was thinking of it being compiles.

Yes, adding tests might be worth it, if we already have the builder. How long does it take to run the given tests?

The Cronet tests run around 5 minutes. If we include net_unittests, it may take extra 8 minutes. The compilation time increases by around 3-4 minutes. This numbers are based on this run: https://build.chromium.org/p/tryserver.chromium.android/builders/android_cronet_tester/builds/1337

 Issue 767101  has been merged into this issue.

jbudorick@ is working on making Cronet tests run on swarming.
John, should we merge this into Issue 662623?

no, though it should be blocked on that.

We should also get Cronet test bot on CQ.

client-specific components are now merged to Infra>Client>Chrome

Context: https://crrev.com/c/723719/4/ios/PRESUBMIT.py#45

A CQ bot is being added through src/ios/PRESUBMIT.py, effectively adding a new trybot for changes in ios/ . IMHO, the bot should really be in the CQ config proper, but that may need more capacity and more approvals.

To help at least the capacity, I vaguely remember CQ being able to trigger builds based on paths or some other conditions - +tandrii@ for more info.

My back-of-the-envelope capacity estimates (see the above link) suggest that we'd only need 2-3 more bots to handle the load, since compilation / tests are guarded by the analyze step.

I've removed my changes from src/ios/PRESUBMIT.py as CQ config sounds like a right approach.

CQ doesn't trigger builders conditionally on paths.
The respective paths' PRESUBMIT.py is where it is done, and it's used extensively through chrome. I have mixed feelings about this, but things will stay like for foreseeable future.

Thanks, tandrii@! In that case, I'm perfectly fine with adding the builder to CQ through ios/PRESUBMIT.py. Sorry if it caused a delay. At least I learned something new from this.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/015b1d87acd07f08391c528a553f8753336bfe1e

commit 015b1d87acd07f08391c528a553f8753336bfe1e
Author: Misha Efimov <mef@chromium.org>
Date: Wed Oct 18 18:49:59 2017

Add Cronet iOS trybot to all iOS changes.

Bug: 712733
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I3a774cc6680a96849a9ce013fb2943361e0abe91
Reviewed-on: https://chromium-review.googlesource.com/726004
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509821}
[modify] https://crrev.com/015b1d87acd07f08391c528a553f8753336bfe1e/ios/PRESUBMIT.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a8baf4905ec918ff5da701e379758daadcac419

commit 9a8baf4905ec918ff5da701e379758daadcac419
Author: Misha Efimov <mef@chromium.org>
Date: Fri Oct 27 22:37:01 2017

Add Cronet trybots to all net changes.

Bug: 712733
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I98565609711612b21d5381c0f60bdcbedc9da33b
Reviewed-on: https://chromium-review.googlesource.com/741886
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512331}
[add] https://crrev.com/9a8baf4905ec918ff5da701e379758daadcac419/net/PRESUBMIT.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab9d5f30a210faa36cff2518cce6a2676016c025

commit ab9d5f30a210faa36cff2518cce6a2676016c025
Author: Miriam Gershenson <mgersh@chromium.org>
Date: Thu Nov 16 18:07:26 2017

Revert "Add Cronet trybots to all net changes."

This reverts commit 9a8baf4905ec918ff5da701e379758daadcac419.

These bots don't have enough capacity to handle all net/ CLs, and
they've been blocking people from landing changes.

Bug: 712733
Change-Id: I270079658a22301553c998acff8ae2395d3e2fa3
Reviewed-on: https://chromium-review.googlesource.com/774958
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517125}
[delete] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/net/PRESUBMIT.py

As a data point, currently the android_cronet_tester bot has 3 slaves (one of which is offline).
Each try run takes about 30-40 minutes.

In the meanwhile we should add Cronet testers to CQ for components/grpc as they setup quic test server, and there is very little churn to trigger capacity limitations.

See  issue 796311  as an example of missing trybot.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08e66f2ba914e4e59ab19bd5bb544b1ef00209d3

commit 08e66f2ba914e4e59ab19bd5bb544b1ef00209d3
Author: Misha Efimov <mef@chromium.org>
Date: Thu Dec 21 17:10:52 2017

Add Cronet trybots to all components/grpc_support changes.

Bug: 712733
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I7dcdb0f66dce16ed91a1a77ca5be40168dd4888c
Reviewed-on: https://chromium-review.googlesource.com/839883
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525722}
[add] https://crrev.com/08e66f2ba914e4e59ab19bd5bb544b1ef00209d3/components/grpc_support/PRESUBMIT.py

It seems that marking this as Fixed is causing confusion. 
There is no Cronet test bot on CQ unconditionally, but there are Cronet test bots added to changes in //ios, //components/grpc_support and //components/cronet.

I don't know how common this is, but I just had a CL reverted that would have been caught if even the gn generation step for cronet was run: https://chromium-review.googlesource.com/c/chromium/src/+/1246833

Actually building/testing it wouldn't have been required, since none of my changes affect targets cronet uses.

If that's something that happens often, then maybe just running that (rather than tests/etc) would be cheap enough, but if my CL is a special case then it may not be worth the trouble there :)

(aside: perhaps cronet doesn't need to parse all the GN files in the first place? it doesn't seem like it has any business objecting to things defined in //chrome)

cronet builds in the android_cronet bot; for some reason, yours got analyzed: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android_cronet/105766

I'm more interested in why didn't that get caught in the non-cronet android builders.

So on further investigation my change is not broken on cronet: it's broken on is_official_build=true, which the cronet waterfall builder is but few/none of our other bots are(?). The trybot either didn't actually run gn, or didn't have that setting enabled.

This was even more confused by the fact that my CL crossed over in the CQ with another, unrelated CL that it logically conflicted with, causing a *second* failure reason not specific to cronet either.