Bug 63959

Summary: garden-o-matic should know how to fetch test results from the (NRWT) bots
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Adam Barth 2011-07-05 16:13:45 PDT
garden-o-matic should know how to fetch test results from the (NRWT) bots
Comment 1 Adam Barth 2011-07-05 16:14:53 PDT
Created attachment 99758 [details]
Patch
Comment 2 Adam Barth 2011-07-05 16:15:31 PDT
@ojan: Can we make the server understand the normal JSONP "callback" parameter?  That would make the client side code cleaner (and more robust).
Comment 3 Ojan Vafai 2011-07-05 16:22:29 PDT
(In reply to comment #2)
> @ojan: Can we make the server understand the normal JSONP "callback" parameter?  That would make the client side code cleaner (and more robust).

That's fine with me. The only issue is the local results.html file needs to load full_results.json from the local filesystem, which as you are well aware we can't do XHR to the local filesystem in Chrome. So, at least for that script, we need the callback built in to the written out JSON. If we're going to have it for that one, may as well have it for all of them?

That said, we could have the server replace the hard-coded call with the callback, but that gains us less.

Open to better suggestions of course. I agree this isn't ideal.
Comment 4 Adam Barth 2011-07-05 16:24:46 PDT
I think it's fine for the server to replace the default callback with one specified in the URL.  That seems like the easiest solution.
Comment 5 Adam Barth 2011-07-05 17:12:31 PDT
Created attachment 99765 [details]
Patch
Comment 6 Eric Seidel (no email) 2011-07-05 17:17:12 PDT
Comment on attachment 99765 [details]
Patch

OK.
Comment 7 Ojan Vafai 2011-07-05 17:18:22 PDT
Comment on attachment 99765 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99765&action=review

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/results.js:15
> +  function fetchResults(builder_name, success) {

nit: I tend to call paramters likes this "onsuccess" or "successHandler" or something that makes it clear it's a function not a boolean.
Comment 8 Ojan Vafai 2011-07-05 17:19:06 PDT
Whoops. Didn't mean to reset the cq+. I'll leave it cq? for now in case you want to address my nit. Feel free to ignore it though.
Comment 9 Adam Barth 2011-07-05 17:25:52 PDT
Created attachment 99766 [details]
Patch
Comment 10 Adam Barth 2011-07-05 17:26:31 PDT
> nit: I tend to call paramters likes this "onsuccess" or "successHandler" or something that makes it clear it's a function not a boolean.

Yeah, I was emulating the name jquey uses in its $.get function, but onsuccess does seem much clearer.  :)
Comment 11 WebKit Review Bot 2011-07-05 18:18:34 PDT
Comment on attachment 99766 [details]
Patch

Clearing flags on attachment: 99766

Committed r90424: <http://trac.webkit.org/changeset/90424>
Comment 12 WebKit Review Bot 2011-07-05 18:18:39 PDT
All reviewed patches have been landed.  Closing bug.