Milo UI report wrong list of commits within a build |
||||||||
Issue descriptionI noticed this issue when investigated issue 864942 . Basically media_perftest was failing, and I initially suspect it was caused by martiniss@'s CL in https://chromium.googlesource.com/chromium/src/+/f53c3ea9907464bf25fef2add0536e402a129851 However, when I look at https://ci.chromium.org/buildbot/chromium.perf/mac-10_13_laptop_high_end-perf/424, I found it included his commit, so I thought maybe it wasn't his. After some local debugging, I confirm it was actually martiniss@'s commit that caused the test failure. But how come the mac-10_13_laptop_high_end-perf/424 was green?? To check whether martiniss's commit was included in the isolate of the build, I fetch one from a shard in media_perftests step in mac-10_13_laptop_high_end-perf/424 build. I run: $ ./tools/swarming_client/isolateserver.py download -I https://chrome-isolated.appspot.com --namespace default-gzip -s cd5b6ae646777ddf8906e6a4c5d5f44b05f3cd58 --target foo $ grep -nr get_chromium_gtest_summary_passes foo/ <no output> So that confirm that the build didn't include martiniss@'s commit (since he added get_chromium_gtest_summary_passes method in https://chromium-review.googlesource.com/c/chromium/src/+/1125420) My opinion is this bug is serious since it will make it a lot harder for people to figure out the culprit of tests failures. +maruel@ in case this is a bug in swarming rather than milo
,
Jul 18
#1: this wasn't a try job, it's a continuous build. And the job ran without including the patch.
,
Jul 18
milo computes blamelist based on build's revision. The revision is 8025aa7e1614a59fb70f1f9589bb1e0d933f656e. It includes f53c3ea9907464bf25fef2add0536e402a129851. bot_update also reports that it checked out 8025aa7e1614a59fb70f1f9589bb1e0d933f656e https://logs.chromium.org/v/?s=chrome%2Fbb%2Fchromium.perf%2Fmac-10_13_laptop_high_end-perf%2F424%2F%2B%2Frecipes%2Fsteps%2Fbot_update%2F0%2Flogs%2Fjson.output%2F0 I don't see how this is a milo bug. The unexpected behavior happened inside the build, not in milo. CCI, PTAL.
,
Jul 18
#4: thanks for the analysis. I wasn't sure which layer caused this bug, so apologize for the mis-evaluation
,
Jul 18
The blamelist calculation here is done by buildbot, Milo is just reporting what buildbot thinks the revision is. Also martiniss@'s CL is supposed to be in build #424, and it is, so I guess i'm not sure what the bug is? I'm closing this as WontFix because in the example given in the OP, the list of commits within the build is in fact, correct, as far as Milo/Buildbot is concerned.
,
Jul 18
#5: the bug is martiniss@'s CL is shown in build #424, but the actual build doesn't include his CL (see the isolate analysis in #0)
,
Jul 19
Ned, this bot's still hosted on Buildbot rather than LUCI, right? If so then you should urgently migrate it to LUCI. The Infra team can't be expected to maintain the obsolete Buildbot infrastructure and LUCI automatically solves a ton of problems – presumably including this one.
,
Sep 10
AFAIK, Buildbot's blamelist support is known to be buggy. I'm going to mark this as Available, but I don't think we're going to get around to fixing this any time soon. If it happens again feel free to comment on this bug.
,
Sep 10
(as an FYI I think this bug sat around for so long because it was Unconfirmed, which our trooper queue monorail search query ignored until recently).
,
Sep 10
I am working on migrating perf builders to LUCI, so feel free to mark this WontFix
,
Sep 10
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mar...@chromium.org
, Jul 18