A question for infra

The first build where this started to happen: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/29777

I'm not yet sure what's going on, and even if it's WAI, it's definitely confusing.

+jbudorick and martiniss - in case you know how test failures are decided to be ignored? (I tried code search, but it goes way too deep...)

That looks like a bug in the recipe

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-xenial-rel/4275

The linux-xenial-builder also shows successful builds with failed steps.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20Tests%20x64%20%28dbg%29 win10 as well

It looks like this has (so far) only been reported for CI bots. I'm taking a look at this.

I bet what's happening is that https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/test_utils/api.py?type=cs&g=0&l=212 is running for trybots, but not CI builds, and so some test suites, like layout tests, aren't reporting all their failures using the return value of the run step.

Compare https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8932284038678635264/+/steps/network_service_browser_tests_on_Ubuntu-16.04/0/stdout#L294528_0 (which fails the build) and https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8932267278337357744/+/steps/webkit_python_tests_on_Ubuntu-16.04/0/stdout#L2958548_0 (which doesn't fail the build). The step which doesn't fail the build doesn't return 1. Not exactly sure what this means, but it semi makes sense. I wrote some tests in recipes, and the recipe does seem to not fail on isolated_script_tests if the retcode = 0.

https://chromium-review.googlesource.com/c/chromium/tools/build/+/1291449/1/scripts/slave/recipe_modules/chromium_tests/tests/api/main_waterfall_steps.py#257 shows the two tests I added. The gtest test fails with no retcode=1, but the isolated_script_test doesn't.

 Issue 897184  has been merged into this issue.

Ok, I found something weird. The swarming tasks themselves aren't failing. 

Look at https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-xenial-rel/4275. The swarming task for webkit_python_tests is https://chromium-swarm.appspot.com/task?id=40a36f825631f810&refresh=10. That task is COMPLETED (SUCCESS). 

So it looks like there's maybe a bug in the actual source side script?

I think this is caused by ned's recent refactorings.

Running //third_party/blink/tools/run_blinkpy_tests.py (with a forced failed test) locally exits with 1, but running `//testing/scripts/run_isolated_script_test.py ./third_party/blink/tools/run_blinkpy_tests.py --isolated-script-test-output out.json` exits with 0. It does print that the command returned an exit code of 1, but the actual exit code of run_isolated_script_test.py returns a 0.

I think this only affects CI bots because tryserver bots look at test results, whereas CI bots rely more on the output of the test step.

Ned, do you think this is related to your refactorings?

Oh, I found the problem. There's a return missing here: https://cs.chromium.org/chromium/src/testing/scripts/run_isolated_script_test.py?q=isolated_script_&sq=package:chromium&g=0&l=83

I'll make a CL

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

commit e45a3de1341e3976b6050640df41a5c5ddce6b8a
Author: Stephen Martinis <martiniss@chromium.org>
Date: Fri Oct 19 20:19:24 2018

//testing/scripts: Add missing returns

It appears that some return statements were accidentally omitted during
an earlier refactor. This CL adds a few of them back.

Bug:  896175 
Change-Id: I218d8ae6fa347fea775d606b5aa04064662ff7e6
Reviewed-on: https://chromium-review.googlesource.com/c/1291787
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601276}
[modify] https://crrev.com/e45a3de1341e3976b6050640df41a5c5ddce6b8a/testing/scripts/run_chromedriver_tests.py
[modify] https://crrev.com/e45a3de1341e3976b6050640df41a5c5ddce6b8a/testing/scripts/run_isolated_script_test.py

 Issue 896936  has been merged into this issue.

This still seems to not be fixed.... https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-xenial-rel/4349 is an example of a broken build that should be red but isn't.  :(

I didn't change this line: https://cs.chromium.org/chromium/src/testing/scripts/run_isolated_script_test.py?q=run_isolated_script_test.py&sq=package:chromium&dr&l=92

Whoops 

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

commit 0865005c35f478f2fe36fdb70241db7fb3a8cb52
Author: Stephen Martinis <martiniss@chromium.org>
Date: Mon Oct 22 18:50:37 2018

run_isolated_script_test.py: Return exit code

My previous CL missed this line.

TBR=nednguyen

Bug:  896175 
Change-Id: I2f37aca90820754caa1b4eb7186b654657d15ac9
Reviewed-on: https://chromium-review.googlesource.com/c/1294206
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601668}
[modify] https://crrev.com/0865005c35f478f2fe36fdb70241db7fb3a8cb52/testing/scripts/run_isolated_script_test.py

Should be fixed for real now, I'll monitor.

Ok, looks to be fixed; https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-xenial-rel/4453 is a recent failure.