New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 723572 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----



Sign in to add a comment

ios/try recipe ignores patch application failure

Project Member Reported by tandrii@chromium.org, May 17 2017

Issue description

The builder had problem applying the patch, but ignored it and continued with compilation, and turned out green.

This has been happening as early as https://luci-milo.appspot.com/buildbot/tryserver.chromium.mac/ios-device/209498
and possibly earlier.
 

Comment 1 by s...@google.com, May 17 2017

Labels: Pri-1
Owner: phajdan.jr@chromium.org
Status: Assigned (was: Untriaged)
iOS applies the patch by calling bot_update.ensure_checkout. We are not try/excepting it. Why isn't patch failure raising StepFailure and halting the build?
Exactly
Cc: d...@chromium.org
Labels: -Restrict-View-Google
Status: Started (was: Assigned)
Uploaded https://chromium-review.googlesource.com/c/508790/ and https://chromium-review.googlesource.com/c/508697/ .

My investigation indicates this regressed in https://chromium-review.googlesource.com/c/475950/ , about a month ago.
Project Member

Comment 5 by bugdroid1@chromium.org, May 18 2017

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

commit f347ca1f531aad4cfb60ac6f23b4a8d8426195db
Author: Paweł Hajdan, Jr <phajdan.jr@chromium.org>
Date: Thu May 18 22:04:37 2017

ios/try: add regression test for patch application failure

Bug:  723572 
Change-Id: I41d8ab750e53e13dc2444c6cf80cc5c45234ab85
Reviewed-on: https://chromium-review.googlesource.com/508697
Commit-Queue: smut <smut@chromium.org>
Reviewed-by: Sergey Berezin <sergeyberezin@chromium.org>
Reviewed-by: smut <smut@chromium.org>

[add] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.expected/patch_failure.json
[modify] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.py

Project Member

Comment 6 by bugdroid1@chromium.org, May 18 2017

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

commit f347ca1f531aad4cfb60ac6f23b4a8d8426195db
Author: Paweł Hajdan, Jr <phajdan.jr@chromium.org>
Date: Thu May 18 22:04:37 2017

ios/try: add regression test for patch application failure

Bug:  723572 
Change-Id: I41d8ab750e53e13dc2444c6cf80cc5c45234ab85
Reviewed-on: https://chromium-review.googlesource.com/508697
Commit-Queue: smut <smut@chromium.org>
Reviewed-by: Sergey Berezin <sergeyberezin@chromium.org>
Reviewed-by: smut <smut@chromium.org>

[add] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.expected/patch_failure.json
[modify] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.py

Project Member

Comment 7 by bugdroid1@chromium.org, May 18 2017

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

commit f347ca1f531aad4cfb60ac6f23b4a8d8426195db
Author: Paweł Hajdan, Jr <phajdan.jr@chromium.org>
Date: Thu May 18 22:04:37 2017

ios/try: add regression test for patch application failure

Bug:  723572 
Change-Id: I41d8ab750e53e13dc2444c6cf80cc5c45234ab85
Reviewed-on: https://chromium-review.googlesource.com/508697
Commit-Queue: smut <smut@chromium.org>
Reviewed-by: Sergey Berezin <sergeyberezin@chromium.org>
Reviewed-by: smut <smut@chromium.org>

[add] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.expected/patch_failure.json
[modify] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.py

Project Member

Comment 8 by bugdroid1@chromium.org, May 18 2017

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

commit f347ca1f531aad4cfb60ac6f23b4a8d8426195db
Author: Paweł Hajdan, Jr <phajdan.jr@chromium.org>
Date: Thu May 18 22:04:37 2017

ios/try: add regression test for patch application failure

Bug:  723572 
Change-Id: I41d8ab750e53e13dc2444c6cf80cc5c45234ab85
Reviewed-on: https://chromium-review.googlesource.com/508697
Commit-Queue: smut <smut@chromium.org>
Reviewed-by: Sergey Berezin <sergeyberezin@chromium.org>
Reviewed-by: smut <smut@chromium.org>

[add] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.expected/patch_failure.json
[modify] https://crrev.com/f347ca1f531aad4cfb60ac6f23b4a8d8426195db/scripts/slave/recipes/ios/try.py

Status: Fixed (was: Started)

Sign in to add a comment