Bug 77250

Summary: webkit-perf.appspot.com should have an ability to merge tests
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: WebKit WebsiteAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, ojan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77037    
Attachments:
Description Flags
Adds admin/merge-tests none

Ryosuke Niwa
Reported 2012-01-27 15:22:08 PST
When tests are moved or renamed, we need to merge multiple Test objects. Provide an interface to do this on an admin page.
Attachments
Adds admin/merge-tests (13.62 KB, patch)
2012-01-27 15:55 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-01-27 15:55:46 PST
Created attachment 124388 [details] Adds admin/merge-tests
WebKit Review Bot
Comment 2 2012-01-27 16:00:21 PST
Attachment 124388 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Websites/webkit-perf.appspot..." exit_code: 1 Websites/webkit-perf.appspot.com/merge_tests_handler.py:45: multiple statements on one line (semicolon) [pep8/E702] [5] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Ryosuke Niwa
Comment 3 2012-01-27 16:02:05 PST
The style error is caused by ";" in content-type http header. Unfortunately, we use a third-party script for python style check so it's non-trivial to fix it.
Ryosuke Niwa
Comment 4 2012-01-30 13:53:59 PST
Ping reviewers. This patch is blocking my patch to add memcache.
Adam Barth
Comment 5 2012-01-30 14:23:07 PST
Comment on attachment 124388 [details] Adds admin/merge-tests I'm not sure what sort of review you're looking for here. Generally, we use PEP8 style in our python (but without the 80 col limit). Names like mergedResults don't match that style, but given that this is an addition to code imported from Mozilla, we should probably match their style. Are you planning to contribute these changes upstream? In any case, I'm inclined to rubber-stamp these sorts of changes.
Ryosuke Niwa
Comment 6 2012-01-30 14:51:17 PST
Thanks for the review. (In reply to comment #5) > (From update of attachment 124388 [details]) > I'm not sure what sort of review you're looking for here. Generally, we use PEP8 style in our python (but without the 80 col limit). Names like mergedResults don't match that style, but given that this is an addition to code imported from Mozilla, we should probably match their style. They use names with underscores? I can change them since all the python code I'm checking into webkit won't be shared with Mozilla. > Are you planning to contribute these changes upstream? No. Python backend is specific to WebKit. In fact, all patches I've contributed so far only contains WebKit-specific code. As I've outlined in http://trac.webkit.org/changeset/105971, we need to import some code from https://github.com/mozilla/graphs but it's all in front-end (css, js, html).
Adam Barth
Comment 7 2012-01-30 14:55:00 PST
> They use names with underscores? I can change them since all the python code I'm checking into webkit won't be shared with Mozilla. Yeah. I'm surprised check-webkit-style didn't complain. (Maybe it stops at the first error in each file?)
Ryosuke Niwa
Comment 8 2012-01-30 15:06:44 PST
Comment on attachment 124388 [details] Adds admin/merge-tests Clearing flags on attachment: 124388 Committed r106292: <http://trac.webkit.org/changeset/106292>
Ryosuke Niwa
Comment 9 2012-01-30 15:06:47 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.