Summary: | Slave lost shouldn't be recognized as build failed | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, eric | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 40055, 40056 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Csaba Osztrogonác
2010-05-18 03:57:58 PDT
Created attachment 56355 [details]
proposed patch
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. 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. Landed in http://trac.webkit.org/changeset/60559 (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. Created attachment 57659 [details]
Patch
(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' Comment on attachment 57659 [details]
Patch
LGTM. This seems like the right fix.
(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 . |