WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
39282
Slave lost shouldn't be recognized as build failed
https://bugs.webkit.org/show_bug.cgi?id=39282
Summary
Slave lost shouldn't be recognized as build failed
Csaba Osztrogonác
Reported
2010-05-18 03:57:58 PDT
Unfortunately slave lost is recognized as build failed now, and sherrifbot bother innocent folks with false positive alarms on IRC and add comments into bugzilla. I propose sherrifbot should handle "slave lost" as "build successful".
Attachments
proposed patch
(1.49 KB, patch)
2010-05-18 03:59 PDT
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
Patch
(3.33 KB, patch)
2010-06-02 08:41 PDT
,
Csaba Osztrogonác
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1
2010-05-18 03:59:38 PDT
Created
attachment 56355
[details]
proposed patch
Eric Seidel (no email)
Comment 2
2010-05-18 04:06:07 PDT
I don't think this is the right fix. But it's an interesting hack. For one, we would need to unit test this. Secondly, I think the right fix would be to crawl back a revision and use that as the current red/green status instead of just assuming "slave-lost" means green. Although maybe we should just treat lost slaves as green? Not sure.
Eric Seidel (no email)
Comment 3
2010-06-01 11:30:27 PDT
Comment on
attachment 56355
[details]
proposed patch Please add a FIXME comment. Something like: # FIXME: We treat slave lost as green even though it is not to work # around the Qts bot being on a broken internet connection. # The real fix is
https://bugs.webkit.org/show_bug.cgi?id=37099
But this solution is pragmatic and we should do it.
Csaba Osztrogonác
Comment 4
2010-06-02 05:13:31 PDT
Landed in
http://trac.webkit.org/changeset/60559
Csaba Osztrogonác
Comment 5
2010-06-02 05:37:49 PDT
(In reply to
comment #4
)
> Landed in
http://trac.webkit.org/changeset/60559
And rolled-out by
http://trac.webkit.org/changeset/60560
because I forgot about unit test and broke the whole world. Sorry. Fix is coming soon.
Csaba Osztrogonác
Comment 6
2010-06-02 08:41:55 PDT
Created
attachment 57659
[details]
Patch
Csaba Osztrogonác
Comment 7
2010-06-02 08:46:28 PDT
(In reply to
comment #6
)
> Created an attachment (id=57659) [details] > Patch
Here is the fixed patch with unit test for slave lost. I have to add a "not not", because False or None == None. Or is there any simple way to fix this problem? Error message why I had to roll-out my earlier patch: Traceback (most recent call last): File "/Volumes/Data/WebKit-BuildSlave/snowleopard-intel-leaks/build/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py", line 218, in test_status_parsing self.assertEquals(builder[key], expected_value, ("Builder %d parse failure for key: %s: Actual='%s' Expected='%s'" % (x, key, builder[key], expected_value))) AssertionError: Builder 2 parse failure for key: is_green: Actual='None' Expected='False'
Eric Seidel (no email)
Comment 8
2010-06-02 11:44:54 PDT
Comment on
attachment 57659
[details]
Patch LGTM. This seems like the right fix.
Csaba Osztrogonác
Comment 9
2010-06-02 13:55:46 PDT
(In reply to
comment #8
)
> (From update of
attachment 57659
[details]
) > LGTM. This seems like the right fix.
Landed in
http://trac.webkit.org/changeset/60575
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug