WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
76745
check-webkit-style of the chromium test_expectations.txt file takes too long
https://bugs.webkit.org/show_bug.cgi?id=76745
Summary
check-webkit-style of the chromium test_expectations.txt file takes too long
Ojan Vafai
Reported
2012-01-20 15:03:05 PST
check-webkit-style of the chromium test_expectations.txt file takes too long
Attachments
Patch
(12.10 KB, patch)
2012-01-20 15:07 PST
,
Ojan Vafai
no flags
Details
Formatted Diff
Diff
Patch
(11.56 KB, patch)
2012-01-20 19:41 PST
,
Ojan Vafai
dglazkov
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Ojan Vafai
Comment 1
2012-01-20 15:07:00 PST
Created
attachment 123386
[details]
Patch
Dimitri Glazkov (Google)
Comment 2
2012-01-20 15:19:04 PST
I think the right approach would be to just reuse the model, instead of caching. The model no longer depends on the port/platform against which you check, so it should just be able to take multiple passes. That's how I envisioned it working, anyway.
Dimitri Glazkov (Google)
Comment 3
2012-01-20 15:19:47 PST
That's why the errors are collected for each line -- so that you could review them all once you ran them against all ports.
Ojan Vafai
Comment 4
2012-01-20 19:41:49 PST
Created
attachment 123428
[details]
Patch
Tony Chang
Comment 5
2012-01-20 23:18:22 PST
Comment on
attachment 123428
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=123428&action=review
I'll let dglazkov do the actual review. Just some style nits.
> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:795 > if len(errors) or len(warnings):
Nit: I think you can remove the len() calls.
> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:799 > if len(errors):
Ditto.
> Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:220 > + def test_error_on_different_platform(self):
Do we have a test that checks different configurations (e.g., DEBUG vs RELEASE)?
Dimitri Glazkov (Google)
Comment 6
2012-01-24 11:33:39 PST
Comment on
attachment 123428
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=123428&action=review
>> Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:220 >> + def test_error_on_different_platform(self): > > Do we have a test that checks different configurations (e.g., DEBUG vs RELEASE)?
I think it might be good to capture just a few more expectations here, just in case the machinery in layout_tests/models is refactored away or something.
Ojan Vafai
Comment 7
2012-01-24 12:10:47 PST
Committed
r105770
: <
http://trac.webkit.org/changeset/105770
>
Vincent Scheib
Comment 8
2012-01-24 13:09:39 PST
Committed
r105789
: <
http://trac.webkit.org/changeset/105789
>
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