New issue
Advanced search Search tips

Issue 891327 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

CL passing chromium CQ, when it should be failing

Project Member Reported by martiniss@chromium.org, Oct 2

Issue description

https://ci.chromium.org/p/chromium/builders/luci.chromium.try/chromeos-amd64-generic-rel/99802 is a sample CL

This CL should be failing the CQ, but it seems to be passing. My suspicion is that this is because of https://crrev.com/c/1249643. It looks to me like if there's a CL which touches a file which disable deapplying the patch, then it will never fail the tryjob run.

I've uploaded https://crrev.com/c/1255559. I added a test in there, chromium_trybot.swarming_test_failure_no_patch_deapplication, which fails without the change in the CL (the exit code is 0).
 
Owner: martiniss@chromium.org
Status: Assigned (was: Unconfirmed)
-> martiniss since you're working on the CL to fix this.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/fadc59d5630bbd88c122521bfb56390e9c5b4ecd

commit fadc59d5630bbd88c122521bfb56390e9c5b4ecd
Author: Stephen Martinis <martiniss@chromium.org>
Date: Tue Oct 02 19:07:20 2018

chromium_tests: Fix non deapplied patch failures

CLs which touch whitelisted files, and which fail tests,
currently don't fail the build, due to a logic bug. This CL
fixes this logic and adds a test.

Bug:  891327 
Change-Id: Ic78b36c9c2b92cb7864e70ab47c93e883f4e7599
Reviewed-on: https://chromium-review.googlesource.com/1255559
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Auto-Submit: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/fadc59d5630bbd88c122521bfb56390e9c5b4ecd/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/nonzero_exit_code_no_gtest_output.json
[add] https://crrev.com/fadc59d5630bbd88c122521bfb56390e9c5b4ecd/scripts/slave/recipes/chromium_trybot.expected/swarming_test_failure_no_patch_deapplication.json
[modify] https://crrev.com/fadc59d5630bbd88c122521bfb56390e9c5b4ecd/scripts/slave/README.recipes.md
[modify] https://crrev.com/fadc59d5630bbd88c122521bfb56390e9c5b4ecd/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/disable_deapply_patch_affected_files.json
[modify] https://crrev.com/fadc59d5630bbd88c122521bfb56390e9c5b4ecd/scripts/slave/recipe_modules/chromium_tests/api.py
[modify] https://crrev.com/fadc59d5630bbd88c122521bfb56390e9c5b4ecd/scripts/slave/recipes/chromium_trybot.py

Need to verify this fix worked.
Status: Fixed (was: Assigned)
https://chromium-review.googlesource.com/c/chromium/src/+/1292728 failed the CQ, so I think this fix landed. I'd like to add some sort of automated test to verify a CL like that always fails, but I'll mark this fixed for now.

Sign in to add a comment