[WPT Export] Exporter builds are green even though there were patches that didn't apply |
||
Issue descriptionhttps://build.chromium.org/p/chromium.infra.cron/builders/wpt-exporter/builds/36368 is an example, from the logs: 2017-08-31 02:23:11,251 - Found Gerrit in-flight CL: "Replace "style-dev@" with "style-dev+owners@" in OWNERS files." https://chromium-review.googlesource.com/c/598831 2017-08-31 02:23:11,256 - No in-flight PR found for CL. Creating... 2017-08-31 02:23:15,747 - Gerrit CL patch did not apply cleanly: 2017-08-31 02:23:15,747 - Failed to run "['git', 'apply', '-']" exit_code: 1 cwd: /tmp/wpt output: error: patch failed: cssom-view/OWNERS:1 error: cssom-view/OWNERS: patch does not apply error: patch failed: cssom/OWNERS:1 error: cssom/OWNERS: patch does not apply error: selectors/OWNERS: No such file or directory 2017-08-31 02:23:15,747 - First 500 characters of patch: << END_OF_PATCH_EXCERPT 2017-08-31 02:23:15,747 - From ea61386c1caeeb73180ebfdc03e40f7729619c44 Mon Sep 17 00:00:00 2001 From: Eddy Mead <meade@chromium.org> Date: Thu, 03 Aug 2017 16:29:16 +1000 Subject: [PATCH] Replace "style-dev@" with "style-dev+owners@" in OWNERS files. This allows subscribers of style-dev to filter out automated CL messages from the WPT import bot. Change-Id: I377560fcdf404006f12bbbaff2bb5c7688e955ec Now, this change shouldn't be exported, which is tracked by issue 759181 . However, the same kind of patch failure could happen with any other file, and if we fail to notice, the change would eventually be reverted.
,
Aug 31 2017
Two issues here. First, regarding the exporter status, it is expected to be green in this case. There are two kinds of exports: 1) from Gerrit in-flight CLs, 2) from Chromium commits. The current implementation only shows red light when 2) fails, which I think is reasonable: If we fail in case 1), we still have a lot of chances to fix up: when a new patchset is uploaded, and finally when the CL is checked in. And there are at least two legitimate reasons for an in-flight CL to fail: a) the patch contains binary changes ( issue 743153 ), b) we are temporarily out of sync and there are merge conflicts now in head, in which case the conflicts should be exposed after the next import and resolved by a rebase (if somehow the author rushes the CL into Chromium, we will eventually find out as it'll be a type-2 failure and the exporter will be red.) Second, the particular patch failure here is a bug ( issue 759181 ), and the fix is already underway. Closing as WONTFIX for now, but please do feel free to comment on/reopen the bug if you have other thoughts on the decision of ignoring in-flight patch errors. Thanks for reporting!
,
Sep 4 2017
Oh, I didn't pay attention to the fact that this was for an in-flight CL. Thanks! |
||
►
Sign in to add a comment |
||
Comment 1 by foolip@chromium.org
, Aug 31 2017