WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
32797
[bzt] Optimize status updates for new dashboard
https://bugs.webkit.org/show_bug.cgi?id=32797
Summary
[bzt] Optimize status updates for new dashboard
Adam Barth
Reported
2009-12-20 10:08:41 PST
We should make the bots post more information about errors and their status so the dashboard can present this information.
Attachments
work in progress
(15.75 KB, patch)
2009-12-20 10:55 PST
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(22.09 KB, patch)
2009-12-20 17:01 PST
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(21.99 KB, patch)
2009-12-20 17:07 PST
,
Adam Barth
eric
: review+
eric
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Adam Barth
Comment 1
2009-12-20 10:55:24 PST
Created
attachment 45286
[details]
work in progress
Eric Seidel (no email)
Comment 2
2009-12-20 11:15:23 PST
Comment on
attachment 45286
[details]
work in progress I'm surprised this isn't an error in python: 45 self._update_status("Building." % (patch["id"], patch["bug_id"]), patch) Personally I like storing state['patch'] in a local patch variable: 74 message = "Attachment %s did not build on %s:\nBuild output: %s" % (state["patch"]["id"], cls.port_name, results_link) Again, should erorr: 230 self._update_status("Checking style." % (patch["id"], patch["bug_id"]), patch) I'm not sure we should even output that much: 248 message = "Attachment %s did not pass %s:\n\n%s" % (state["patch"]["id"], cls.name, script_error.message_with_output(output_limit=3*1024)) You could re-use an OutputCapture object: 50 OutputCapture().assert_outputs(self, queue.queue_log_path, expected_stdout=expected_stdout.get("queue_log_path", ""), expected_stderr=expected_stderr.get("queue_log_path", "")) The test seems kinda confusing... but OK.
Adam Barth
Comment 3
2009-12-20 11:24:54 PST
> I'm surprised this isn't an error in python:
Oh, it probably does. I wasn't posting it for review. Hopefully the tests will find these bugs though. :)
> Personally I like storing state['patch'] in a local patch variable:
Ok.
> I'm not sure we should even output that much: > 248 message = "Attachment %s did not pass %s:\n\n%s" % > (state["patch"]["id"], cls.name, > script_error.message_with_output(output_limit=3*1024))
Hum... Pick a number. :)
> The test seems kinda confusing... but OK.
Yeah, I thought of a better way to do this. Basically, we want to make a MockQueueEngine to do all the callbacks. :) Anyway, I've got to run. Thanks for the comments.
Adam Barth
Comment 4
2009-12-20 17:01:19 PST
Created
attachment 45301
[details]
Patch
WebKit Review Bot
Comment 5
2009-12-20 17:03:18 PST
style-queue ran check-webkit-style on
attachment 45301
[details]
without any errors.
Adam Barth
Comment 6
2009-12-20 17:07:08 PST
Created
attachment 45302
[details]
Patch
WebKit Review Bot
Comment 7
2009-12-20 17:08:42 PST
style-queue ran check-webkit-style on
attachment 45302
[details]
without any errors.
Eric Seidel (no email)
Comment 8
2009-12-20 18:42:44 PST
Comment on
attachment 45302
[details]
Patch Should this have a period? 45 self._update_status("Building.", patch) The expected_stderr repeition in EarlyWarningSytemTest could be avoided by using a shared function. Said shared function could also pre-compute what the expected absolute path should be for the begin_work_queue message. The relative path change I think makes it harder to read. The purpose of that message was to make sure that I was in the right directory before I let the commit-queue loose on my working directory. Maybe the message isn't as useful anymore now that these run most commonly on bots? I'd prefer we pre-computed the absolute path in the unittests than make this relative: 83 log("CAUTION: %s will discard all local changes in \"%s\"" % (self.name, os.path.relpath(self.tool.scm().checkout_root))) Why not just do: engine(foo, bar).run() on one line here: work_queue = QueueEngine(self.name, self) 118 work_queue = engine(self.name, self) 108119 return work_queue.run() I like the did_pass, did_fail changes! assert_queue_outputs could be re-written much more consisely. If you don't plan to do it in this patch, please add a FIXME for us to do it later. Please make some of the above adjustments before landing.
Eric Seidel (no email)
Comment 9
2009-12-28 22:41:12 PST
Attachment 45302
[details]
was posted by a committer and has review+, assigning to Adam Barth for commit.
Eric Seidel (no email)
Comment 10
2009-12-28 23:43:39 PST
I'm shocked an appalled! Already landed as
r52430
. Closing.
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