Bug 36195

Summary: Add layout test flakiness (diagnostics) dashboard to TestResultServer appengine
Product: WebKit Reporter: Victor Wang <victorw>
Component: Tools / TestsAssignee: Victor Wang <victorw>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, dglazkov, ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Proposed patch
none
Proposed patch - remove extra empty line
none
Proposed Patch
abarth: review-
Patch per comment #4
none
Correct app name and remove a few unused lines.
none
Remove CRs in index.yaml none

Description Victor Wang 2010-03-16 14:52:01 PDT
Host layout test flakiness dashboard in TestResultServer appengine. Flakiness dashboard is a tool (used by chromium) to monitor layout test status and help diagnose layout test regressions.
Comment 1 Victor Wang 2010-03-16 15:34:43 PDT
Created attachment 50845 [details]
Proposed patch
Comment 2 Victor Wang 2010-03-16 15:39:32 PDT
Created attachment 50846 [details]
Proposed patch - remove extra empty line
Comment 3 Victor Wang 2010-03-23 16:10:15 PDT
Created attachment 51466 [details]
Proposed Patch
Comment 4 Adam Barth 2010-03-24 00:41:18 PDT
Comment on attachment 51466 [details]
Proposed Patch

+ self.response.out.write(files[0].data)

Mime type?  Character set?

+ self.response.out.write("\n".join(errors))

XSS?

+ DashboardFile.delete_file(file)

CSRF?  Public delete file URL?

+ http://src.chromium.org/

Wrong project?

+ url = SVN_PATH_DASHBOARD + name

URL encoding?

Tests???
Comment 5 Victor Wang 2010-03-24 14:45:54 PDT
Created attachment 51545 [details]
Patch per comment #4

Thanks for your good inputs, see my comments inline.

(In reply to comment #4)
> (From update of attachment 51466 [details])
> + self.response.out.write(files[0].data)
> 
> Mime type?  Character set?
done

> 
> + self.response.out.write("\n".join(errors))
> 
> XSS?
ya, thanks for spotting this. Actually no need to write errors to response.out. The error messages is set in response.status. Removed.


> 
> + DashboardFile.delete_file(file)
> 
> CSRF?  Public delete file URL?
Added code to only allow AE admins to do this and only shows the "delete" links in file list for admin account.

> 
> + http://src.chromium.org/
> 
> Wrong project?
For now, the flakiness dashboard source code is still in chromium tree as it has code that are shared with other chromium tests. I think we will either move them upstream or have an upstream version so that non-chromium port can be benefit from this tool.


> 
> + url = SVN_PATH_DASHBOARD + name
> 
> URL encoding?
done

> 
> Tests???
There is a JAVA version of AE test framework but no python version yet (confirmed with Google AE team). I prefer waiting until a python version is released instead of making my own version for AE testing like datastore, blobstore, handler etc, thoughts? I could file a bug for adding test for AppEngine once it is ready if needed...
Comment 6 Victor Wang 2010-03-24 14:49:34 PDT
Created attachment 51547 [details]
Correct app name and remove a few unused lines.
Comment 7 Adam Barth 2010-03-25 16:06:09 PDT
Comment on attachment 51547 [details]
Correct app name and remove a few unused lines.

Thanks for addressing my feedback.  I'm not super excited about the dependency on the chromium source server, but I don't see a better solution in the meantime.

We have another AppEngine app in QueueStatusServer, which also doesn't have any test coverage.  I'm deathly afraid of touching it for that reason.  A testing framework would certainly be nice.  :)
Comment 8 Victor Wang 2010-03-25 17:40:08 PDT
Created attachment 51700 [details]
Remove CRs in index.yaml

Looks like mac-ews does not like CRs in index.yaml (generated by AE while I am running on windows), remove CRs in index.yaml. Hope this will make mac-ews happy.
Comment 9 WebKit Commit Bot 2010-03-26 11:14:51 PDT
Comment on attachment 51700 [details]
Remove CRs in index.yaml

Clearing flags on attachment: 51700

Committed r56635: <http://trac.webkit.org/changeset/56635>
Comment 10 WebKit Commit Bot 2010-03-26 11:14:58 PDT
All reviewed patches have been landed.  Closing bug.