Bug 136711

Summary: Merge some Blink webkitpy Changes
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bfulgham, buildbot, commit-queue, dbates, dfarler, glenn, ossy, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 136727    
Bug Blocks: 136722    
Attachments:
Description Flags
Patch
none
Correct merge error.
ossy: review-, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion none

Brent Fulgham
Reported 2014-09-10 13:11:12 PDT
The Windows port has experienced numerous failures in the webkitpy system due to improper mixing of unicode and ascii data in the string handling (and elsewhere). In an effort to clear this up, I am bringing our webkitpy libraries in closer sync with Blink's implementation. This is a first patch that covers a variety of subsections. Subsequent patches will deal with specific subsystems.
Attachments
Patch (196.62 KB, patch)
2014-09-10 13:40 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Correct merge error. (196.20 KB, patch)
2014-09-10 15:12 PDT, Brent Fulgham 		ossy: review-
buildbot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (532.39 KB, application/zip)
2014-09-10 16:46 PDT, Build Bot 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2014-09-10 13:40:33 PDT
Created attachment 237904 [details] Patch
Brent Fulgham
Comment 2 2014-09-10 15:12:33 PDT
Created attachment 237907 [details] Correct merge error.
Build Bot
Comment 3 2014-09-10 16:45:58 PDT
Comment on attachment 237907 [details] Correct merge error. Attachment 237907 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5578578913656832 New failing tests: http/tests/media/video-throttled-load-metadata.html http/tests/media/video-served-as-text.html http/tests/media/video-error-does-not-exist.html
Build Bot
Comment 4 2014-09-10 16:46:01 PDT
Created attachment 237917 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Brent Fulgham
Comment 5 2014-09-10 17:01:08 PDT
I don't see these http video failures locally. According to the logs, these are all "notifyDone" failures, which means the page probably got hung or crashed somewhere. It doesn't seem likely that these webkitpy changes would cause just these video HTTP tests to fail; I would expect all HTTP tests to fail if there was a new bug.
Alexey Proskuryakov
Comment 6 2014-09-10 20:12:01 PDT
Yes, these failures are unrelated, as they happened on multiple patches today.
Csaba Osztrogonác
Comment 7 2014-09-11 05:41:03 PDT
Comment on attachment 237907 [details] Correct merge error. View in context: https://bugs.webkit.org/attachment.cgi?id=237907&action=review Merging many different fixes in one huge patch isn't a good idea. Additionally we should add references to the original changes and describe the reason why we need a fix to be merged. > Tools/Scripts/webkitpy/tool/grammar.py:41 > -def pluralize(count, noun, showCount=True): > +def pluralize(noun, count, showCount=True): I don't agree with this change. It was modified intentionally to be be more understandable, please don't change it back. To get "2 patches" now, you can simple do it with pluralize(2, "patch"). Why do you think if pluralize("patch", 2) would be better< To be closer to Blink's webkitpy isn't a good reason.
Csaba Osztrogonác
Comment 8 2014-09-11 05:55:19 PDT
Comment on attachment 237907 [details] Correct merge error. View in context: https://bugs.webkit.org/attachment.cgi?id=237907&action=review > Tools/Scripts/webkitpy/layout_tests/views/printing.py:316 > - summary = "%s ran as expected, %d didn't%s:" % (grammar.pluralize(expected, "test"), unexpected, incomplete_str) > + summary = "%s ran as expected%s, %d didn't%s%s:" % (grammar.pluralize('test', expected), expected_summary_str, unexpected, incomplete_str, timing_summary) Changing the output of the run-webkit-tests would break the buildbots. The result parser of run-webkit-test in master.cfg should be changed to handle the new format properly. And the buildmaster should be restarted too.
Brent Fulgham
Comment 9 2014-09-30 12:08:47 PDT
This seems like the wrong approach. Closing!
Note You need to log in before you can comment on or make changes to this bug.