Use always_run v can_fail_build

Use always_run v can_fail_build, and make compile a critical failure

TBR=dpranke@chromium.org,kbr@chromium.org,agable@chromium.org,machenbach@chromium.org,sivachandra@chromium.org
BUG=286398
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=224593

[Ken Russell (switch to Gerrit), 2013-09-20 22:49:01]

LGTM if this is the right flag.

Do you want to mention BUG=286398?

[iannucci, 2013-09-20 22:52:18]

On 2013/09/20 22:49:01, Ken Russell wrote:
> LGTM if this is the right flag.
> 
> Do you want to mention BUG=286398?

Yeah. The only thing I'm iffy about is that the build will be in the 'failed'
state as soon as one of these fails, which means that any steps in the recipe
afterwards will be skipped unless they're manually marked as 'always_run' as
well.

[Ken Russell (switch to Gerrit), 2013-09-20 22:54:25]

On 2013/09/20 22:52:18, iannucci wrote:
> On 2013/09/20 22:49:01, Ken Russell wrote:
> > LGTM if this is the right flag.
> > 
> > Do you want to mention BUG=286398?
> 
> Yeah. The only thing I'm iffy about is that the build will be in the 'failed'
> state as soon as one of these fails, which means that any steps in the recipe
> afterwards will be skipped unless they're manually marked as 'always_run' as
> well.

Are those the semantics the old system used to have? It would be better if all
steps always ran unless a failing step was specifically marked as being able to
*abort* the build. Then the build would succeed or fail depending on whether any
step failed.

[iannucci, 2013-09-21 16:21:00]

I've +'d a bunch of people for FYI.

Previously:
  * All steps run until any step fails which sets the build into a 'failed'
state
  * From this point on, we keep trying to get steps from GenSteps until we hit
an error (usually the recipe will try to access data on a step which was
skipped, resulting in an exception). If we get steps marked as 'always_run',
then we'll run them, otherwise we skip them.

Attempted fix #1:
  * Make all test steps 'can_fail_build=False'. This prevents the build from
going into a failed state, continuing to run tests. However it also means that
the overall build status will be green when it should be red due to failed
tests.

This fix:
  * Make all test steps 'always_run=True'. This allows tests to fail the build,
but subsequent tests will still run. However, this also implies that it will try
to run tests even if the compile step fails.
  * To counter this, I've marked the compile step as abort_on_failure=True,
which will cause the entire build to terminate early if there's a compile
failure. I can't think of a circumstance where we would actually want the build
to continue on a failed compile anyway.

These control-flow flags are pretty messy, I think. For example, you cannot
write cleanup steps to run after a compile failure with this patch. We have some
ideas on how to clean this up, but it involves some heavy lifting. In the new
model, you'll be able to do more complex flows without needing all these special
flags (most likely, you would be able catch/finally the RecipeAbort exception
and yield some more steps).

I'm TBR'ing this for now to make the gpu waterfall actually show coverage, since
they're flying blind at the moment (Everything is green no matter what!), and
kbr is OOO until Tuesday. I'll keep an eye on these other waterfalls to make
sure they don't turn any new sorts of colors :).

[Michael Achenbach, 2013-09-21 16:22:41]

lgtm for v8 part

[commit-bot: I haz the power, 2013-09-21 16:24:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iannucci@chromium.org/24311004/10001

[commit-bot: I haz the power, 2013-09-21 16:24:53]

Failed to apply patch for
build/scripts/slave/recipes/gpu.expected/linux_debug.json:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file build/scripts/slave/recipes/gpu.expected/linux_debug.json
  Hunk #5 FAILED at 127.
  1 out of 5 hunks FAILED -- saving rejects to file
build/scripts/slave/recipes/gpu.expected/linux_debug.json.rej

Patch:       build/scripts/slave/recipes/gpu.expected/linux_debug.json
Index: scripts/slave/recipes/gpu.expected/linux_debug.json
diff --git build/scripts/slave/recipes/gpu.expected/linux_debug.json
build/scripts/slave/recipes/gpu.expected/linux_debug.json
index
274220fa01d63bab10095d0c95ef6564380d3d1a..e0cdfbb92bb2efaf4e0b1f3dfc81d83f3ddb2085
100644
--- a/build/scripts/slave/recipes/gpu.expected/linux_debug.json
+++ b/build/scripts/slave/recipes/gpu.expected/linux_debug.json
@@ -49,6 +49,7 @@
     "name": "gclient runhooks"
   },
   {
+    "abort_on_failure": true,
     "cmd": [
       "python",
       "-u",
@@ -69,7 +70,7 @@
     "name": "compile"
   },
   {
-    "can_fail_build": false,
+    "always_run": true,
     "cmd": [
       "python",
       "-u",
@@ -88,7 +89,7 @@
     "name": "content_gl_tests"
   },
   {
-    "can_fail_build": false,
+    "always_run": true,
     "cmd": [
       "python",
       "-u",
@@ -107,7 +108,7 @@
     "name": "gles2_conform_test"
   },
   {
-    "can_fail_build": false,
+    "always_run": true,
     "cmd": [
       "python",
       "-u",
@@ -126,7 +127,7 @@
     "name": "gl_tests"
   },
   {
-    "can_fail_build": false,
+    "always_run": true,
     "cmd": [
       "python",
       "-u",

[commit-bot: I haz the power, 2013-09-21 16:29:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iannucci@chromium.org/24311004/17001

[commit-bot: I haz the power, 2013-09-21 16:31:04]

Change committed as 224593