WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
136711
Merge some Blink webkitpy Changes
https://bugs.webkit.org/show_bug.cgi?id=136711
Summary
Merge some Blink webkitpy Changes
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
Details
Formatted Diff
Diff
Correct merge error.
(196.20 KB, patch)
2014-09-10 15:12 PDT
,
Brent Fulgham
ossy
: review-
buildbot
: commit-queue-
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug